Skip to content

add prefix to network settings for dynamic and vip network#2641

Merged
fmoehler merged 3 commits intomainfrom
add-prefix-to-network-settings-for-dynamic-and-vip-network
Nov 20, 2025
Merged

add prefix to network settings for dynamic and vip network#2641
fmoehler merged 3 commits intomainfrom
add-prefix-to-network-settings-for-dynamic-and-vip-network

Conversation

@fmoehler
Copy link
Copy Markdown
Contributor

@fmoehler fmoehler commented Nov 18, 2025

What is this change about?

This PR will add the prefix to the network settings of dynamic and vip networks which was previously missed. Although these network types do not support prefix allocation (yet) they should have this property as well to make all network types (manual, dynamic, vip) have the same config.

Please provide contextual information.

This issue was revealed after 9ec3b74#r170570246 was merged.

What tests have you run against this PR?

Reran previously failing integration tests + unit tests.

How should this change be described in bosh release notes?

Set prefix in network_settings of dynamic and vip networkds

Does this PR introduce a breaking change?

No

Tag your pair, your PM, and/or team!

@neddp , @Ivaylogi98 , @mariash

@github-project-automation github-project-automation bot moved this from Inbox to Pending Merge | Prioritized in Foundational Infrastructure Working Group Nov 18, 2025
@aramprice
Copy link
Copy Markdown
Member

aramprice commented Nov 18, 2025

Manually running integration specs:

Specs are still running but it looks like there are some failures around ordering of IP's in expected return values. Not sure if this ordering is important but it appears to have changed. An partial excerpt from the output of the spec run above:

Changing ip ranges
  shifting the IP range for a job
      should fail to create the link
      and the link is shared from another deployment
              should create new link
            when provider networks change
              default network stays the same
                new network added to cloud config and deployment
        should not create new vms
      when using instances with static ip addresses
      Re-uploads all packages to replace old ones and eliminates broken compiled packages
    when uploading compiled package
      returns error when conflicting with another custom definition
    when manifest has non-conflicting custom provider definitions
> Spec failed: ./spec/integration/global_networking/vip_networks_spec.rb:53
> Test tmpdir: /tmp/build/e55deab7/bosh/src/tmp/integration-specs/sandbox-pid-17801/tmp/spec-20251118-17801-9c5ycb

expected: ["192.168.1.2", "69.69.69.69"]
     got: ["69.69.69.69", "192.168.1.2"]

(compared using ==)

> ---------------
    reuses instance vip network IP on subsequent deploy (FAILED - 1)
      runs the errand if the instances selected change
    does not stop jobs after the errand has run
  errand name != first job
        should fail to create the link
  when the consumer implicitly consumes a link
    when there are multiple providers providing a link and one of the providers is set to nil
          should create link with IP for cross-deployment link and DNS for implicit link
    when provider use_dns_address is FALSE
      Re-uploads all compiled packages to replace old ones

vm state
    should recreate VMs outside of the range in the new range, but not touch VMs that are ok

checking link properties
  should update trusted certificates when the VM is first created
      respects default properties and should create link successfully
    when release job requires and provides same link
                  creates new link using existing consumer for external link request for new network
> Spec failed: ./spec/integration/global_networking/vip_networks_spec.rb:73
> Test tmpdir: /tmp/build/e55deab7/bosh/src/tmp/integration-specs/sandbox-pid-17801/tmp/spec-20251118-17801-fzd3wz

expected: ["192.168.1.2", "69.69.69.69"]
     got: ["69.69.69.69", "192.168.1.2"]

(compared using ==)

> ---------------
    shows no change on update (FAILED - 2)
  when the operator predefines vips in the cloud config
    releases its IP on subsequent deploy
  when vm_type is specified for compilation
        renders with the same value
      when deployment also specifies use_short_dns_addresses
        should not create-swap-deleted vms
  when a create-swap-delete deployment fails with unresponsive agent on a link provider VM
      renders link data in job template
    when provide and consume links are set in spec, but only implied by deployment manifest
    should run the requested bin/run on the first instance
  when instance lifecycle = service
    running with /first
  should not raise an error when consuming links without properties
      can resolve the links

network lifecycle
  disabled
    when deploying a manifest with a manual network not marked as managed
      should create link with IP

Managed persistent disk
                  creates new link using existing consumer for external link request for existing network
              default network changes
                requesting external link for new network
  should not update the trusted certificates if they were not changed
    should use the cloud_properties from the compilation vm_type

Aliasing links to DNS addresses
  when configuring aliases to release links
        uses DNS address
        when instance is recreated
> Spec failed: ./spec/integration/global_networking/vip_networks_spec.rb:129
> Test tmpdir: /tmp/build/e55deab7/bosh/src/tmp/integration-specs/sandbox-pid-17801/tmp/spec-20251118-17801-r311oy

expected: ["192.168.1.2", "69.69.69.69"]
     got: ["69.69.69.69", "192.168.1.2"]

(compared using ==)

> ---------------
    reuses instance vip network IP on subsequent deploy (FAILED - 3)
  changes a single instance group instance state when referenced by id
  detached
  should not raise an error when a deployment template property is not defined in the release properties
      runs the errand if the configuration on the selected instances change
      should not attempt to create a subnet in the iaas
  enabled
      runs the errand on the first instance (ordered by uuid)
  when running an errand across multiple instances
      renders link data in job template
    multiple provide links with same type
      when both provided links are on separate templates
    updates DNS addresses on dependent consumer VMs
  when running dry run initially
    provides both aliases
  when configuring aliases to custom provided links
  should be able to resolve a manual configuration in a consumes link

Finished in 89 minutes 38 seconds (files took 2.96 seconds to load)
69 examples, 0 failures

                  creates new link and uses same consumer record
                requesting external link for old network
  mounts a persistent disk at /store in the agent base dir
  when the agent is restarted
        raises error before deploying vms
      when both provided links are in same template
> Spec failed: ./spec/integration/global_networking/vip_networks_spec.rb:151
> Test tmpdir: /tmp/build/e55deab7/bosh/src/tmp/integration-specs/sandbox-pid-17801/tmp/spec-20251118-17801-ygkvw6

expected: ["192.168.1.2", "69.69.69.69"]
     got: ["69.69.69.69", "192.168.1.2"]

(compared using ==)

> ---------------
    updates when the cloud config is changed (FAILED - 4)

a-hassanin
a-hassanin previously approved these changes Nov 18, 2025
Copy link
Copy Markdown
Contributor

@a-hassanin a-hassanin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@aramprice aramprice left a comment

Choose a reason for hiding this comment

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

@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Nov 18, 2025
@fmoehler fmoehler force-pushed the add-prefix-to-network-settings-for-dynamic-and-vip-network branch from 7f158eb to 2806791 Compare November 19, 2025 13:20
@fmoehler
Copy link
Copy Markdown
Contributor Author

fmoehler commented Nov 20, 2025

@aramprice I reran the integration tests and now they are green: https://bosh.ci.cloudfoundry.org/builds/291139895 did not know that you can run those standalone otherwise would have done that before ;)

@beyhan beyhan moved this from Waiting for Changes | Open for Contribution to Pending Review | Discussion in Foundational Infrastructure Working Group Nov 20, 2025
@github-project-automation github-project-automation bot moved this from Pending Review | Discussion to Pending Merge | Prioritized in Foundational Infrastructure Working Group Nov 20, 2025
@aramprice
Copy link
Copy Markdown
Member

@fmoehler - thanks this looks good. I think go ahead and merge so we can get the pipeline back in working order.

@fmoehler fmoehler merged commit 2e880ab into main Nov 20, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

5 participants