Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cni): test #12590

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft

fix(cni): test #12590

wants to merge 21 commits into from
+444 −387

Conversation

bartsmykla
Copy link
Contributor

IN PROGRESS: just want to check stability

Motivation

Implementation information

Supporting documentation

Fix #XX

@bartsmykla
refactor(cni): unhardcode file paths in install logic
f0b8c80 
Replaced hardcoded paths in CNI install functions with parameters for
better flexibility. Updated `prepareKubeconfig` and `prepareKumaCniConfig`
to accept token and CA certificate paths as arguments. Adjusted constant
definitions for service account paths in `main.go` and refactored related
logic accordingly

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): simplify error handling in install functions
6ec5cfd 
Simplified error handling in CNI installation functions by using inline
checks for atomic writes and permission changes. Replaced redundant error
assignments with direct conditionals. Improved clarity and reduced
boilerplate in `Run`, `install`, and related functions.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): rename receiver in InstallerConfig validation
d299627 
Renamed the receiver in `InstallerConfig.Validate` from `i` to `c` for
better consistency.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): improve error messages in validation logic
5d7a4e8 
Enhanced error messages in `checkInstall` to include more context and
specificity. Updated file existence checks to provide file paths in errors.
Improved clarity in chained and standalone plugin validation errors by
adding details about expected formats and missing configurations.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): enhance validation error messages and logging
6723755 
Improved error messages in validation functions for better clarity and
context. Updated `isValidConfFile` and `isValidConflistFile` to provide
specific details about missing fields. Enhanced logging to better describe
validation results and failures for configuration and conflist files.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): improve error handling and logging in main logic
8274bbf 
Refined error handling in `main.go` to enhance clarity and reduce redundancy.
Replaced multierr with `errors.Join` for better error aggregation. Improved
logging messages for binary copying and config reversion to include more
context. Adjusted function parameters for better consistency.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): add context support and improve error handling
9629acf 
Introduced context propagation in installation and setup functions for
better control over operations. Enhanced error messages across various
functions to provide more clarity and context. Updated file and path
handling to use `filepath` for consistency and robustness.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): fix tests
e11459b 
Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): small cleanup/refactor in findCniConfFile
c2d8de1 
Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): change receiver to pointer in InstallerConfig methods
9738836 
Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): move cni config discovery to post-processing step
589a1c3 
Refactored CNI config file discovery logic into a `PostProcess` method
within `InstallerConfig`. This simplifies the `loadInstallerConfig`
function and ensures config file detection is handled consistently. Added
logging to improve visibility into file discovery and fallback behavior.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
test(cni): update installer config tests for post-process logic
317988d 
Updated tests for `InstallerConfig` to include `PostProcess` method
validation. Changed paths in test cases to use `filepath` for better
compatibility

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): restructure kubeconfig generation and add tests
c2c3680 
Moved kubeconfig preparation logic into `InstallerConfig` methods for
better encapsulation. Added `GenerateKubeconfigTemplate`.

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): simplify kubeconfig handling and config preparation
da81cc9 
Changed `PrepareKubeconfig` and `PrepareKumaCniConfig` to take token data
directly instead of reading from a file. Made config preparation simpler
with better logging. Updated tests and golden files to match changes

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): move install checks to InstallerConfig and update tests
285a39c 
Moved `checkInstall` logic into `InstallerConfig.CheckInstall` for better
encapsulation. Updated runtime and test code to use the new method. Removed
the old `checkInstall` function and related validation tests

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla
refactor(cni): simplify installer configuration and preparation
2ff0951 
Updated `InstallerConfig` to include default paths for service account
token and CA certificate. Simplified `PrepareKubeconfig` and `PrepareKumaCniConfig`
methods to read required files internally. Adjusted main installation logic
to check existing configurations before proceeding with installation. Updated
tests to reflect the changes

Signed-off-by: Bart Smykla <bartek@smykla.com>
@bartsmykla bartsmykla added ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) ci/verify-stability labels Jan 18, 2025
@github-actions GitHub Actions
Copy link
Contributor

Reviewer Checklist

🔍 Each of these sections need to be checked by the reviewer of the PR 🔍:
If something doesn't apply please check the box and add a justification if the reason is non obvious.

  • Is the PR title satisfactory? Is this part of a larger feature and should be grouped using > Changelog?
  • PR description is clear and complete. It Links to relevant issue as well as docs and UI issues
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as an image registry)
  • IPv6 is taken into account (.e.g: no string concatenation of host port)
  • Tests (Unit test, E2E tests, manual test on universal and k8s)
    • Don't forget ci/ labels to run additional/fewer tests
  • Does this contain a change that needs to be notified to users? In this case, UPGRADE.md should be updated.
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) ci/verify-stability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bartsmykla