nv-ipam CNI plugin #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
moshe010
reviewed
May 2, 2023
ykulazhenkov
reviewed
May 2, 2023
adrianchiris
force-pushed
the
nv-ipam
branch
2 times, most recently
from
5dc9a3a
to
1580524
Compare
|
Unit-tests and possible refactoring for them will be added as a followup PR. @moshe010 @ykulazhenkov PTAL |
moshe010
reviewed
May 3, 2023
pkg/cni/plugin/plugin.go
Outdated
| func (p *Plugin) CmdDel(args *skel.CmdArgs) error { | ||
| conf, err := types.LoadConf(args.StdinData) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to load config: %w", err) |
There was a problem hiding this comment.
you log print is inconsistent is it
: %w or . %w ?
pkg/cni/plugin/plugin.go
Outdated
| if err != nil { | ||
| return fmt.Errorf("failed to marshal host-local net conf. %w", err) | ||
| } | ||
| log.Debugf("host-local stdin data:%q", string(data)) |
| DefaultKubeConfigFileName = "nv-ipam.kubeconfig" | ||
| // ConfFileName is the name of CNI configuration file found in conf dir | ||
| ConfFileName = "nv-ipam.conf" | ||
| ) |
There was a problem hiding this comment.
comment to self refactor code to use constant once this PR is merged
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read k8s node name from path: %s. %w", p, err) | ||
| } | ||
| n.IPAM.NodeName = strings.TrimSpace(string(data)) |
There was a problem hiding this comment.
the trim is unnecessary but it not hurt to do it
| func LoadFromConfFile(filePath string) (*IPAMConf, error) { | ||
| data, err := os.ReadFile(filePath) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
maybe change err to fmt.Errorf("Failed to ReadFile %s: %v", filePath, err)
| confFromFile := &IPAMConf{} | ||
| err = json.Unmarshal(data, confFromFile) | ||
| if err != nil { | ||
| return nil, err |
Signed-off-by: adrianc <adrianc@nvidia.com>
types package contains CNI types for nv-ipam and host-local Signed-off-by: adrianc <adrianc@nvidia.com>
k8sclient package is used to create a kubernetes client from kubeconfig file provided by path in filesystem Signed-off-by: adrianc <adrianc@nvidia.com>
pool package is used to manage pool configuration as defined in node annotation Signed-off-by: adrianc <adrianc@nvidia.com>
cmd add flow updated Signed-off-by: adrianc <adrianc@nvidia.com>
- use network name as pool name if pool name not provided - use kubeconfig from ConfDir if not provided instead of DefaultConfDir Signed-off-by: adrianc <adrianc@nvidia.com>
- Rename PoolConfig to just Pool - Add pool name to pool Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
This allows to provide per node default configuraiton for nv-ipam cni plugin Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
moshe010
approved these changes
May 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds nv-ipam CNI plugin.
Note: this is still WIP, additional commits will be added