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.

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

libnetwork: allow new none ipam driver #967

Merged
merged 5 commits into from
Mar 29, 2022

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Mar 17, 2022

Network create now uses the ipam driver. This allows the user to
configure the ipam driver manually instead of choosing a fixed default.
If the ipam driver is none no ips will be assigned to this container.
This means that only the interfaces are created.

This will require a patch in netavark since it rejects the config when
no static ips are provided.

Ref containers/podman#13521

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 17, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rhatdan
Copy link
Member

rhatdan commented Mar 18, 2022

LGTM
But need to fix lint issue.

@Luap99
Copy link
Member Author

Luap99 commented Mar 18, 2022

/hold
created a podman PR containers/podman#13553 to make sure there are no regressions

@rhatdan
Copy link
Member

rhatdan commented Mar 23, 2022

Looks like this needs a rebase (Podman PR anyways.)

Luap99 added a commit to Luap99/netavark that referenced this pull request Mar 28, 2022
Allow creating interfaces only with no ip addresses assigned.

Ref containers/common#967

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/netavark that referenced this pull request Mar 28, 2022
Allow creating interfaces only with no ip addresses assigned.
I combined the macvlan and bridge code to prevent unessesary duplication
and bugs.

Ref containers/common#967

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/netavark that referenced this pull request Mar 28, 2022
Allow creating interfaces only with no ip addresses assigned.
Also combine some duplicated code for bridge/macvlan.

Ref containers/common#967

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/netavark that referenced this pull request Mar 28, 2022
Allow creating interfaces only with no ip addresses assigned.
Also combine some duplicated code for bridge/macvlan.

Ref containers/common#967

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added a commit to Luap99/netavark that referenced this pull request Mar 29, 2022
Allow creating interfaces only with no ip addresses assigned.
Also combine some duplicated code for bridge/macvlan.

Ref containers/common#967

Signed-off-by: Paul Holzinger <[email protected]>
Luap99 added 5 commits March 29, 2022 18:46
Parse no ipam plugin and display it as ipam driver none.
Also set the ipam driver field for unsupported plugins.

Signed-off-by: Paul Holzinger <[email protected]>
Network create now uses the ipam driver. This allows the user to
configure the ipam driver manually instead of choosing a fixed default.
If the ipam driver is `none` no cni ipam plugin will be added to teh
config. This means that the interfaces are created but no extra ip
addresses are assigned.

Signed-off-by: Paul Holzinger <[email protected]>
When we read the cni result we should loop over the interfaces and then
the ips. If we only loop over ips we will miss interfaces that have no
ips assigned. We also only care about interfaces created in the netns.

This is required for ipam driver none case, see the test case.

Signed-off-by: Paul Holzinger <[email protected]>
Network create now uses the ipam driver. This allows the user to
configure the ipam driver manually instead of choosing a fixed default.
If the ipam driver is `none` no ips will be assigned to this container.
This means that only the interfaces are created.

This will require a patch in netavark since it rejects the config when
no static ips are provided.

Ref containers/podman#13521

Signed-off-by: Paul Holzinger <[email protected]>
The gocyclo linter is complaining that the cyclomatic complexity is to
high for `(*cniNetwork).createCNIConfigListFromNetwork()`. Split out
option parsing to a new funtion should reduce the complexity.

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99
Copy link
Member Author

Luap99 commented Mar 29, 2022

@baude @mheon @flouthoc PTAL

@mheon
Copy link
Member

mheon commented Mar 29, 2022

LGTM

@mheon
Copy link
Member

mheon commented Mar 29, 2022

/lgtm
/hold cancel

@openshift-merge-robot openshift-merge-robot merged commit 5816258 into containers:main Mar 29, 2022
@Luap99 Luap99 deleted the ipam-none branch March 29, 2022 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants