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

Add Docker network label if custom ipam config #2400

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cywang117
Copy link
Contributor

@cywang117 cywang117 commented Jan 29, 2025

In a target release where the only change is the addition or removal of a custom ipam config, the Supervisor does not recreate the network due to ignoring ipam config differences when comparing current and target network (in network.isEqualConfig). This commit implements the addition of a network label if the target compose object includes a network with custom ipam. With the label, the Supervisor will detect a difference between a network with a custom ipam and a network without, without needing to compare the ipam configs themselves.

This is a major change, as devices running networks with custom ipam configs
will have their services & networks recreated to add the network label.

Closes: #2251
Change-type: major

Release Notes

This release resolves a bug where a target release with only a network custom IPAM config change does not get applied by the Supervisor.

When receiving a target release with a custom IPAM config, the Supervisor will add the io.balena.private.ipam.config label to Docker networks with custom IPAM configs. As a result, these networks will be properly recreated.

This is a major change. Devices that currently have services using networks with custom IPAM configurations will experience:

  • A one-time service restart for services dependent on the custom network
  • Recreation of affected networks to apply proper IPAM configuration

@cywang117 cywang117 force-pushed the network-custom-ipam-label branch from ad0b4a3 to 102ab21 Compare January 29, 2025 12:32
@cywang117
Copy link
Contributor Author

On-device testing

Given two releases where the only delta is a network config change, one without custom ipam config:

version: '2.4'
networks:
  test: {}
services:
  one:
    image: alpine
    command: sh -c "echo one && sleep infinity"
    networks:
      - test
    stop_signal: SIGKILL
  two:
    image: alpine
    command: sh -c "echo two && sleep infinity"
    networks:
      - test
    stop_signal: SIGKILL

and one with custom ipam config:

version: '2.4'
networks:
  test:
    ipam:
      config:
        - subnet: 172.28.0.0/16
          gateway: 172.28.5.254
      driver: default
services:
  one:
    image: alpine
    command: sh -c "echo one && sleep infinity"
    networks:
      - test
    stop_signal: SIGKILL
  two:
    image: alpine
    command: sh -c "echo two && sleep infinity"
    networks:
      - test
    stop_signal: SIGKILL

Engine network metadata is as follows. First, for the release without custom ipam:

root@nuc:~# balena network inspect 33ae5368342d4c5c9807baa7fd1b4de0_test
[
    {
        "Name": "33ae5368342d4c5c9807baa7fd1b4de0_test",
        "Id": "c48e6546119efcc9f82d021d699193f24cb120da88f28ab46cf70f306c8efd30",
        "Created": "2025-01-29T12:30:46.222195521Z",
        "Scope": "local",
        "Driver": "bridge",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "default",
            "Options": {},
            "Config": [
                {
                    "Subnet": "172.25.0.0/16",
                    "Gateway": "172.25.0.1"
                }
            ]
        },
        "Internal": false,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": ""
        },
        "ConfigOnly": false,
        "Containers": {...(unrelated metadata truncated for brevity)...},
        "Options": {},
        "Labels": {
            "io.balena.app-id": "2120892",
            "io.balena.supervised": "true"
        }
    }
]

and for the release with custom ipam:

root@nuc:~# balena network inspect 33ae5368342d4c5c9807baa7fd1b4de0_test
[
    {
        "Name": "33ae5368342d4c5c9807baa7fd1b4de0_test",
        "Id": "7f197ea593c31002d3dd4b2a052e6139e3c0ec99995cb0b3cbfb55ca30c4e690",
        "Created": "2025-01-29T12:31:52.816178006Z",
        "Scope": "local",
        "Driver": "bridge",
        "EnableIPv6": false,
        "IPAM": {
            "Driver": "default",
            "Options": {},
            "Config": [
                {
                    "Subnet": "172.28.0.0/16",
                    "Gateway": "172.28.5.254"
                }
            ]
        },
        "Internal": false,
        "Attachable": false,
        "Ingress": false,
        "ConfigFrom": {
            "Network": ""
        },
        "ConfigOnly": false,
        "Containers": {...(unrelated metadata truncated for brevity)...},
        "Options": {},
        "Labels": {
            "io.balena.app-id": "2120892",
            "io.balena.network.ipam": "true",
            "io.balena.supervised": "true"
        }
    }
]

Note the presence of the io.balena.network.ipam=true label indicating the presence of a custom ipam config. The metadata stays consistent when pinning back & forth between custom and non-custom ipam releases.

@cywang117 cywang117 marked this pull request as ready for review January 29, 2025 12:36
@cywang117 cywang117 requested a review from pipex January 29, 2025 12:36
@flowzone-app flowzone-app bot enabled auto-merge January 29, 2025 16:20
// changes, this label informs the Supervisor that
// there's an ipam diff that requires recreating the network.
if (net.config.ipam.config.length > 0) {
net.config.labels['io.balena.network.ipam'] = 'true';
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like us to use theio.balena.private namespace, I would prefer something like io.balena.private.ipam.config perhaps? io.balena.private.network.ipam works too, although the network part might not totally be necessary since we are already configuring networks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to io.balena.private.ipam.config!

Copy link
Contributor

@pipex pipex left a comment

Choose a reason for hiding this comment

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

I only left one minor comment.

Since this is a major update, could you make sure to write some detailed release notes on the PR body so those get published to the balenaCloud release

If you add a ## Release Notes section, flowzone will use that for release notes on GH and bC

@cywang117 cywang117 force-pushed the network-custom-ipam-label branch from 102ab21 to 0750d4a Compare February 6, 2025 11:42
@cywang117
Copy link
Contributor Author

Thanks for taking a look @pipex ! I updated per your suggestions, and also added some release notes above.

In a target release where the only change is the addition or removal
of a custom ipam config, the Supervisor does not recreate the network
due to ignoring ipam config differences when comparing current and target
network (in network.isEqualConfig). This commit implements the addition of
a network label if the target compose object includes a network with custom
ipam. With the label, the Supervisor will detect a difference between a
network with a custom ipam and a network without, without needing to compare
the ipam configs themselves.

This is a major change, as devices running networks with custom ipam configs
will have their networks recreated to add the network label.

Closes: #2251
Change-type: major
See: https://balena.fibery.io/Work/Project/Supervisor-maintenance-work-(Dec-2024)-948
Signed-off-by: Christina Ying Wang <[email protected]>
@cywang117 cywang117 force-pushed the network-custom-ipam-label branch from 0750d4a to 9afb7ef Compare February 6, 2025 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistencies in network configuration state apply
2 participants