Skip to content

OCPBUGS-77307: Generate KubeVirt nmstate network config conditionally #8365

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

Closed
qinqon wants to merge 3 commits into from
Closed

OCPBUGS-77307: Generate KubeVirt nmstate network config conditionally #8365

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25e6c64
fix(kubevirt): OCPBUGS-77307: override nmstate config for multus with…
qinqon Jun 22, 2026
f516213
fix(e2e): set NodePoolReplicas=1 for KubeVirt TestNodePool like OpenS…
qinqon Jun 22, 2026
74d845f
test(kubevirt): add e2e nmstate network config verification for multi…
qinqon Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions hypershift-operator/controllers/nodepool/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
cpomanifests "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests"
kvinfra "github.com/openshift/hypershift/hypershift-operator/controllers/nodepool/kubevirt"
"github.com/openshift/hypershift/support/api"
"github.com/openshift/hypershift/support/backwardcompat"
"github.com/openshift/hypershift/support/capabilities"
Expand Down Expand Up @@ -212,9 +213,55 @@ func (cg *ConfigGenerator) generateMCORawConfig(ctx context.Context, caps *hyper
configs = append(configs, nodeTuningGeneratedConfigs...)
}

// Generate platform-specific MachineConfigs.
platformConfigs, err := cg.getPlatformConfigs()

@enxebre enxebre Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this would cause a fleet wide nodepool rollout as you upgrade the HO

@qinqon qinqon Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We are going to need to change MC to fix the bug, maybe we can make it opt-in somehow or detect the specific scenario (hosted cluster with non cluster default network and ipv6)

@qinqon qinqon Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reworked to address this — the HyperShift operator no longer generates anything for default-network NodePools: the MCO templates remain the source of truth, so the raw config and hash are byte-identical to before and upgrading the HO does not trigger any rollout. TestGetPlatformConfigs now asserts this hash-neutrality explicitly.

The override MachineConfig is only emitted for NodePools using multus as primary network and whose HostedCluster networking includes IPv6 — exactly the broken population, where the rollout is the fix itself. IPv4-only multus clusters are also left untouched (asymptomatic, and since networking CIDRs are immutable they can never become affected).

@qinqon qinqon Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@enxebre I have limit it to add-default-network=false and ipv6 do make sense now ?

An alternative is to just document this so customers change their nodepoos with machine config deleting those files.

if err != nil {
return "", err
}
configs = append(configs, platformConfigs...)

return cg.parse(configs)
}

// getPlatformConfigs returns platform-specific MachineConfig ConfigMaps
// based on the NodePool's platform configuration.
func (cg *ConfigGenerator) getPlatformConfigs() ([]corev1.ConfigMap, error) {
var rawConfig string
var err error

switch cg.nodePool.Spec.Platform.Type {
case hyperv1.KubevirtPlatform:
rawConfig, err = cg.kubevirtPlatformConfig()
}
if err != nil {
return nil, fmt.Errorf("failed to generate platform config: %w", err)
}

if rawConfig == "" {
return nil, nil
}

return []corev1.ConfigMap{
{
Data: map[string]string{
TokenSecretConfigKey: rawConfig,
},
},
}, nil
}

// kubevirtPlatformConfig generates KubeVirt-specific MachineConfig content.
// For NodePools using multus as primary network (AttachDefaultNetwork=false) on
// clusters with IPv6 networking, it generates an override that replaces the
// MCO-rendered nmstate files (which assume the default pod network) with no-op
// content, allowing standard network auto-configuration (SLAAC) to work.
// For every other NodePool nothing is generated, keeping the NodePool config
// hash unchanged so that upgrading the HyperShift operator does not trigger a
// rollout.
func (cg *ConfigGenerator) kubevirtPlatformConfig() (string, error) {
return kvinfra.GenerateNetworkOverrideMachineConfig(cg.nodePool, cg.hostedCluster.Spec.Networking)
}

// getUserConfigs returns a slice with all the configMaps in nodePool.Spec.Config.
func (cg *ConfigGenerator) getUserConfigs(ctx context.Context) ([]corev1.ConfigMap, error) {
var errors []error
Expand Down
182 changes: 182 additions & 0 deletions hypershift-operator/controllers/nodepool/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1922,6 +1922,188 @@ spec:
}
}

func TestGetPlatformConfigs(t *testing.T) {
ipv4Networking := hyperv1.ClusterNetworking{
ClusterNetwork: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("10.132.0.0/14")}},
ServiceNetwork: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.31.0.0/16")}},
}
dualStackNetworking := hyperv1.ClusterNetworking{
ClusterNetwork: []hyperv1.ClusterNetworkEntry{
{CIDR: *ipnet.MustParseCIDR("10.132.0.0/14")},
{CIDR: *ipnet.MustParseCIDR("fd01::/48")},
},
ServiceNetwork: []hyperv1.ServiceNetworkEntry{
{CIDR: *ipnet.MustParseCIDR("172.31.0.0/16")},
{CIDR: *ipnet.MustParseCIDR("fd02::/112")},
},
}

testCases := []struct {
name string
nodePool *hyperv1.NodePool
networking hyperv1.ClusterNetworking
expectConfigs bool
expectError bool
}{
{
// No config may ever be generated for default-network NodePools:
// anything emitted here becomes part of the NodePool config hash and
// would trigger a fleet-wide rollout when the HyperShift operator is
// upgraded.
name: "When platform is KubeVirt with default network on a dual-stack cluster it should return no configs to keep the config hash unchanged",
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.KubevirtPlatform,
Kubevirt: &hyperv1.KubevirtNodePoolPlatform{
// AttachDefaultNetwork defaults to true when nil
},
},
},
},
networking: dualStackNetworking,
expectConfigs: false,
},
{
name: "When platform is KubeVirt with multus primary network on an IPv4-only cluster it should return no configs to keep the config hash unchanged",
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.KubevirtPlatform,
Kubevirt: &hyperv1.KubevirtNodePoolPlatform{
AttachDefaultNetwork: boolPtr(false),
AdditionalNetworks: []hyperv1.KubevirtNetwork{
{Name: "ns1/localnet-net"},
},
},
},
},
},
networking: ipv4Networking,
expectConfigs: false,
},
{
name: "When platform is KubeVirt with multus primary network on a dual-stack cluster it should return the override config",
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.KubevirtPlatform,
Kubevirt: &hyperv1.KubevirtNodePoolPlatform{
AttachDefaultNetwork: boolPtr(false),
AdditionalNetworks: []hyperv1.KubevirtNetwork{
{Name: "ns1/localnet-net"},
},
},
},
},
},
networking: dualStackNetworking,
expectConfigs: true,
},
{
name: "When platform is AWS it should return no configs",
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.AWSPlatform,
},
},
},
networking: dualStackNetworking,
expectConfigs: false,
},
{
name: "When platform is KubeVirt with nil Kubevirt spec it should return no configs",
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.KubevirtPlatform,
},
},
},
networking: dualStackNetworking,
expectConfigs: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

cg := &ConfigGenerator{
nodePool: tc.nodePool,
hostedCluster: &hyperv1.HostedCluster{
Spec: hyperv1.HostedClusterSpec{
Networking: tc.networking,
},
},
}

configs, err := cg.getPlatformConfigs()
if tc.expectError {
g.Expect(err).To(HaveOccurred())
return
}
g.Expect(err).ToNot(HaveOccurred())

if tc.expectConfigs {
g.Expect(configs).ToNot(BeNil())
g.Expect(configs).To(HaveLen(1))
g.Expect(configs[0].Data).To(HaveKey(TokenSecretConfigKey))
g.Expect(configs[0].Data[TokenSecretConfigKey]).ToNot(BeEmpty())
} else {
g.Expect(configs).To(BeNil())
}
})
}
}

func TestKubevirtPlatformConfig(t *testing.T) {
// The generation matrix (multus/default network, IPv4/dual-stack, non-KubeVirt,
// nil spec) is covered by network_test.go's TestGenerateNetworkOverrideMachineConfig,
// and the ConfigMap wiring is covered by TestGetPlatformConfigs (which calls this
// method). This test only asserts the wiring: the HostedCluster networking is passed
// through, so a multus NodePool on a dual-stack cluster yields the no-op override.
g := NewWithT(t)

cg := &ConfigGenerator{
nodePool: &hyperv1.NodePool{
Spec: hyperv1.NodePoolSpec{
Platform: hyperv1.NodePoolPlatform{
Type: hyperv1.KubevirtPlatform,
Kubevirt: &hyperv1.KubevirtNodePoolPlatform{
AttachDefaultNetwork: boolPtr(false),
AdditionalNetworks: []hyperv1.KubevirtNetwork{
{Name: "ns1/localnet-net"},
},
},
},
},
},
hostedCluster: &hyperv1.HostedCluster{
Spec: hyperv1.HostedClusterSpec{
Networking: hyperv1.ClusterNetworking{
ClusterNetwork: []hyperv1.ClusterNetworkEntry{
{CIDR: *ipnet.MustParseCIDR("10.132.0.0/14")},
{CIDR: *ipnet.MustParseCIDR("fd01::/48")},
},
},
},
},
}

result, err := cg.kubevirtPlatformConfig()
g.Expect(err).ToNot(HaveOccurred())
// The override MachineConfig replacing the MCO-rendered nmstate files is generated.
g.Expect(result).To(ContainSubstring("01-kubevirt-network"))
g.Expect(result).To(ContainSubstring("001-nmstate-disable-ipv6-autoconf"))
g.Expect(result).To(ContainSubstring("002-nmstate-arp-proxy-ipv6-gw"))
}

func boolPtr(b bool) *bool {
return &b
}

func TestGlobalConfigString(t *testing.T) {
expectedGlobalConfigStringWhenEmpty := `{"metadata":{"name":"cluster","creationTimestamp":null},"spec":{"trustedCA":{"name":""}},"status":{}}
{"metadata":{"name":"cluster","creationTimestamp":null},"spec":{"additionalTrustedCA":{"name":""},"registrySources":{}},"status":{}}
Expand Down
Loading
Loading