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

update: add support for netavark update command #503

Merged
merged 1 commit into from
Dec 5, 2022
Merged

update: add support for netavark update command #503

merged 1 commit into from
Dec 5, 2022

Conversation

flouthoc
Copy link
Collaborator

Netavark update allows container managers to update network scoped DNS servers of any configured network and notify running aarvark-dns about it.

netavark-update
Updates network dns servers for an already configured network

USAGE:
    netavark update --network-dns-servers <NETWORK_DNS_SERVERS> <NETWORK_NAME>

ARGS:
    <NETWORK_NAME>    Network name to update

OPTIONS:
    -h, --help                                         Print help information
    -n, --network-dns-servers <NETWORK_DNS_SERVERS>    DNS Servers to update for the network

@flouthoc
Copy link
Collaborator Author

Don't review it yet since it needs #497 otherwise tests will fail.

@flouthoc flouthoc marked this pull request as draft November 28, 2022 01:37
@flouthoc flouthoc mentioned this pull request Nov 28, 2022
Merged
@flouthoc flouthoc force-pushed the netavark-network-update branch 2 times, most recently from b6d191d to 5ee55ca Compare November 29, 2022 03:34
@flouthoc flouthoc force-pushed the netavark-network-update branch from 5ee55ca to 0760e71 Compare November 29, 2022 03:35
@flouthoc flouthoc marked this pull request as ready for review November 29, 2022 03:36
@flouthoc flouthoc force-pushed the netavark-network-update branch 2 times, most recently from af1168c to 6b7cdb3 Compare November 29, 2022 04:12
@flouthoc
Copy link
Collaborator Author

@Luap99 @mheon @baude PTAL

Luap99
Luap99 reviewed Nov 29, 2022
View reviewed changes
src/commands/update.rs Outdated Show resolved Hide resolved
src/commands/update.rs Outdated Show resolved Hide resolved
src/commands/update.rs Outdated Show resolved Hide resolved
src/commands/update.rs Outdated Show resolved Hide resolved
src/dns/aardvark.rs Outdated Show resolved Hide resolved
test/100-bridge-iptables.bats
Comment on lines +115 to +116
# get a random port directly to avoid low ports e.g. 53 would not create iptables
dns_port=$((RANDOM+10000))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not relevant to the test, just remove it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not understand this bit, we are just getting a random port for the test since we are still starting the aardvark.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean don't pass anything and default to 53 ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but now that I look at it again I think this is fine since we should make sure the changed dns port still works.

src/dns/aardvark.rs Outdated Show resolved Hide resolved
@flouthoc
update: add support for netavark update command
0c61f05 
Netavark update allows container managers to update network scoped DNS
servers of any configured network and notify running `aarvark-dns` about
it.

```console
netavark-update
Updates network dns servers for an already configured network

USAGE:
    netavark update --network-dns-servers <NETWORK_DNS_SERVERS> <NETWORK_NAME>

ARGS:
    <NETWORK_NAME>    Network name to update

OPTIONS:
    -h, --help                                         Print help information
    -n, --network-dns-servers <NETWORK_DNS_SERVERS>    DNS Servers to update for the network
```

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc force-pushed the netavark-network-update branch from 6b7cdb3 to 0c61f05 Compare November 29, 2022 12:00
@flouthoc flouthoc requested a review from Luap99 November 29, 2022 12:03
Luap99
Luap99 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Member

@Luap99 Luap99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Nov 29, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flouthoc, 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

@flouthoc
Copy link
Collaborator Author

@baude @mheon PTAL

@flouthoc flouthoc requested review from mheon and baude November 29, 2022 18:09
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 1, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
b09d620 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc mentioned this pull request Dec 1, 2022
Merged
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 1, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
094d3ef 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 1, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
5c77d6c 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 2, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
03f9b1e 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 2, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
ea17a7e 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 3, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
f714653 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
@flouthoc flouthoc mentioned this pull request Dec 4, 2022
Merged
@baude
Copy link
Member

baude commented Dec 5, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 5, 2022
@openshift-merge-robot openshift-merge-robot merged commit f7186d3 into containers:main Dec 5, 2022
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 6, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
db42c65 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 6, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
f69f85c 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 6, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
b62be2b 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/common that referenced this pull request Dec 7, 2022
@flouthoc
libnetwork: extend API to support NetworkUpdate
403bec6 
New features in netavark/aardvark allows users to update network dns
servers and all the containers attached to those networks uses updated
resolvers.

Following PR adds support in `libnetwork` to support

* Add support for higher level `NetworkUpdate` API
* Add support for `Update` exec call which can invoke netavark with a
  new update option added here: containers/netavark#503

Signed-off-by: Aditya R <[email protected]>
flouthoc added a commit to flouthoc/podman that referenced this pull request Jan 12, 2023
@flouthoc
network: add support for podman network update and --network-dns-server
882cd17 
* Add support for `podman network update <>`

```console
network update

Description:
  update networks for containers and pods

Usage:
  podman network update [options] NAME

Examples:
  podman network update podman1

Options:
      --dns-add stringArray      add network level nameservers
      --dns-drop stringArray   remove network level nameservers
```

* Add support for `--network-dns-server` to `podman network create`

Extends podman to support recently added features in `netavark` and
`aardvark-dns`

* containers/netavark#497
* containers/aardvark-dns#252
* containers/netavark#503

[NO NEW TESTS NEEDED]
[NO TESTS NEEDED]

Signed-off-by: Aditya R <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Luap99 Luap99 Luap99 approved these changes

@mheon mheon Awaiting requested review from mheon

@baude baude Awaiting requested review from baude

Assignees

@baude baude

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@flouthoc @baude @Luap99 @openshift-merge-robot