diff --git a/hypershift-operator/controllers/nodepool/config.go b/hypershift-operator/controllers/nodepool/config.go index 1e13e3d61d72..bfdbf4a14bef 100644 --- a/hypershift-operator/controllers/nodepool/config.go +++ b/hypershift-operator/controllers/nodepool/config.go @@ -13,6 +13,7 @@ import ( hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" cpomanifests "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/manifests" + kvnetwork "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" @@ -204,9 +205,55 @@ func (cg *ConfigGenerator) generateMCORawConfig(ctx context.Context, caps *hyper configs = append(configs, nodeTuningGeneratedConfigs...) } + // Generate platform-specific MachineConfigs. + platformConfigs, err := cg.getPlatformConfigs() + 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 kvnetwork.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 diff --git a/hypershift-operator/controllers/nodepool/config_test.go b/hypershift-operator/controllers/nodepool/config_test.go index 558d7e474023..dc3efa76e050 100644 --- a/hypershift-operator/controllers/nodepool/config_test.go +++ b/hypershift-operator/controllers/nodepool/config_test.go @@ -23,6 +23,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" crclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -1941,6 +1942,179 @@ 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 + }{ + { + // 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: ptr.To(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: ptr.To(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() + 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: ptr.To(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 TestGlobalConfigString(t *testing.T) { expectedGlobalConfigStringWhenEmpty := `{"metadata":{"name":"cluster","creationTimestamp":null},"spec":{"trustedCA":{"name":""}},"status":{}} {"metadata":{"name":"cluster","creationTimestamp":null},"spec":{"additionalTrustedCA":{"name":""},"registrySources":{}},"status":{}} diff --git a/hypershift-operator/controllers/nodepool/kubevirt/network.go b/hypershift-operator/controllers/nodepool/kubevirt/network.go new file mode 100644 index 000000000000..e656e2097b66 --- /dev/null +++ b/hypershift-operator/controllers/nodepool/kubevirt/network.go @@ -0,0 +1,159 @@ +package kubevirt + +import ( + "fmt" + "net" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/control-plane-operator/controllers/hostedcontrolplane/ignition" + "github.com/openshift/hypershift/support/api" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + + "github.com/clarketm/json" + ignitionapi "github.com/coreos/ignition/v2/config/v3_2/types" + "github.com/vincent-petithory/dataurl" +) + +const ( + kubevirtNetworkMachineConfigName = "01-kubevirt-network" + ignitionVersion = "3.2.0" + + // Paths must match the nmstate filenames the MCO renders for KubeVirt nodes. + // If MCO renames or adds files, update these so the override keeps covering them. + nmstateDisableIPv6AutoconfPath = "/etc/nmstate/001-nmstate-disable-ipv6-autoconf.yml" + nmstateArpProxyIPv6GwPath = "/etc/nmstate/002-nmstate-arp-proxy-ipv6-gw.yml" +) + +// GenerateNetworkOverrideMachineConfig generates a MachineConfig that overrides +// the MCO-rendered KubeVirt nmstate files with no-op content. +// +// The MCO templates unconditionally render nmstate configuration that disables +// IPv6 autoconf and routes IPv6 through KubeVirt's ARP proxy gateway (fe80::1). +// That configuration is only correct for the default pod network, where +// OVN-Kubernetes assigns IPv6 addresses via DHCPv6. When a NodePool uses multus +// as its primary network (AttachDefaultNetwork=false), the network behaves as a +// standard L2 segment and that configuration breaks SLAAC, preventing nodes from +// getting IPv6 addresses. +// +// The override is only generated when both conditions hold: +// - The NodePool uses multus as primary network (AttachDefaultNetwork=false). +// - The HostedCluster networking includes IPv6 (dual-stack or IPv6 primary). +// +// IPv4-only clusters are excluded on purpose: the stale nmstate files are +// asymptomatic there, and since cluster networking CIDRs are immutable those +// clusters can never become affected. Skipping them keeps this MachineConfig +// out of their NodePool config hash, avoiding a NodePool rollout when the +// HyperShift operator is upgraded. +func GenerateNetworkOverrideMachineConfig(nodePool *hyperv1.NodePool, networking hyperv1.ClusterNetworking) (string, error) { + if nodePool == nil { + return "", nil + } + + if nodePool.Spec.Platform.Type != hyperv1.KubevirtPlatform { + return "", nil + } + + kvPlatform := nodePool.Spec.Platform.Kubevirt + if kvPlatform == nil { + return "", nil + } + + // Only generate the override when using multus (not the default pod network). + if shouldAttachDefaultNetwork(kvPlatform) { + return "", nil + } + + // Only generate the override when the cluster networking includes IPv6. + if !hasIPv6Network(networking) { + return "", nil + } + + noopContent := []byte("# Network configuration not needed for multus primary network\ndesiredState: {}\n") + + return encodeIgnitionAsMachineConfig( + []ignitionapi.File{ + fileFromBytes(nmstateDisableIPv6AutoconfPath, noopContent), + fileFromBytes(nmstateArpProxyIPv6GwPath, noopContent), + }, + "kubevirt network override", + ) +} + +// hasIPv6Network returns true when any of the cluster, service or machine +// networks contains an IPv6 CIDR. +func hasIPv6Network(networking hyperv1.ClusterNetworking) bool { + for _, entry := range networking.ClusterNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + for _, entry := range networking.ServiceNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + for _, entry := range networking.MachineNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + return false +} + +// fileFromBytes creates an ignition-config file with the given contents. +func fileFromBytes(path string, contents []byte) ignitionapi.File { + mode := 0644 + return ignitionapi.File{ + Node: ignitionapi.Node{ + Path: path, + Overwrite: ptr.To(true), + }, + FileEmbedded1: ignitionapi.FileEmbedded1{ + Mode: &mode, + Contents: ignitionapi.Resource{ + Source: ptr.To(dataurl.EncodeBytes(contents)), + }, + }, + } +} + +// encodeIgnitionAsMachineConfig builds a MachineConfig containing the given ignition +// files and returns it as a YAML-encoded string. The label parameter is used in +// error messages to identify the caller context. +func encodeIgnitionAsMachineConfig(files []ignitionapi.File, label string) (string, error) { + ignConfig := ignitionapi.Config{ + Ignition: ignitionapi.Ignition{ + Version: ignitionVersion, + }, + Storage: ignitionapi.Storage{ + Files: files, + }, + } + + serializedConfig, err := json.Marshal(&ignConfig) + if err != nil { + return "", fmt.Errorf("failed to serialize %s ignition config: %w", label, err) + } + + mc := &mcfgv1.MachineConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: kubevirtNetworkMachineConfigName, + }, + } + ignition.SetMachineConfigLabels(mc) + mc.Spec.Config.Raw = serializedConfig + + mc.APIVersion = mcfgv1.SchemeGroupVersion.String() + mc.Kind = "MachineConfig" + + encoded, err := api.CompatibleYAMLEncode(mc, api.YamlSerializer) + if err != nil { + return "", fmt.Errorf("failed to serialize %s machine config: %w", label, err) + } + + return string(encoded), nil +} diff --git a/hypershift-operator/controllers/nodepool/kubevirt/network_test.go b/hypershift-operator/controllers/nodepool/kubevirt/network_test.go new file mode 100644 index 000000000000..8a7c450035a1 --- /dev/null +++ b/hypershift-operator/controllers/nodepool/kubevirt/network_test.go @@ -0,0 +1,265 @@ +package kubevirt + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1" + "github.com/openshift/hypershift/api/util/ipnet" + + mcfgv1 "github.com/openshift/api/machineconfiguration/v1" + + "k8s.io/apimachinery/pkg/util/yaml" + "k8s.io/utils/ptr" + + ignitionapi "github.com/coreos/ignition/v2/config/v3_2/types" + "github.com/vincent-petithory/dataurl" +) + +// decodeIgnitionFileContents parses the YAML MachineConfig, extracts the ignition +// config from Spec.Config.Raw, and returns the concatenated decoded file contents. +func decodeIgnitionFileContents(configYAML string) (string, error) { + mc := &mcfgv1.MachineConfig{} + if err := yaml.NewYAMLOrJSONDecoder(strings.NewReader(configYAML), 4096).Decode(mc); err != nil { + return "", err + } + if mc.Spec.Config.Raw == nil { + return "", fmt.Errorf("MachineConfig Spec.Config.Raw is nil") + } + + ignConfig := &ignitionapi.Config{} + if err := json.Unmarshal(mc.Spec.Config.Raw, ignConfig); err != nil { + return "", err + } + + var decoded strings.Builder + for _, f := range ignConfig.Storage.Files { + if f.Contents.Source == nil { + continue + } + du, err := dataurl.DecodeString(*f.Contents.Source) + if err != nil { + return "", fmt.Errorf("failed to decode ignition file contents for %q: %w", f.Node.Path, err) + } + decoded.Write(du.Data) + } + return decoded.String(), nil +} + +func ipv4Networking() hyperv1.ClusterNetworking { + return hyperv1.ClusterNetworking{ + ClusterNetwork: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("10.132.0.0/14")}}, + ServiceNetwork: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.31.0.0/16")}}, + } +} + +func dualStackNetworking() hyperv1.ClusterNetworking { + return 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")}, + }, + } +} + +func multusPrimaryNodePool() *hyperv1.NodePool { + return &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtNodePoolPlatform{ + AttachDefaultNetwork: ptr.To(false), + AdditionalNetworks: []hyperv1.KubevirtNetwork{ + {Name: "ns1/localnet-net"}, + }, + }, + }, + }, + } +} + +func TestGenerateNetworkOverrideMachineConfig(t *testing.T) { + tests := []struct { + name string + nodePool *hyperv1.NodePool + networking hyperv1.ClusterNetworking + expectConfig bool + expectYAMLContent []string + expectDecodedContent []string + }{ + { + name: "When the NodePool uses multus as primary network on a dual-stack cluster, it should generate the override MachineConfig", + nodePool: multusPrimaryNodePool(), + networking: dualStackNetworking(), + expectConfig: true, + expectYAMLContent: []string{ + "01-kubevirt-network", + "001-nmstate-disable-ipv6-autoconf", + "002-nmstate-arp-proxy-ipv6-gw", + }, + expectDecodedContent: []string{ + "desiredState: {}", + }, + }, + { + name: "When the NodePool uses multus as primary network on an IPv4-only cluster, it should not generate any config", + nodePool: multusPrimaryNodePool(), + networking: ipv4Networking(), + expectConfig: false, + }, + { + name: "When the NodePool uses multus as primary network and only the machine network has IPv6, it should generate the override MachineConfig", + nodePool: multusPrimaryNodePool(), + networking: hyperv1.ClusterNetworking{ + ClusterNetwork: []hyperv1.ClusterNetworkEntry{{CIDR: *ipnet.MustParseCIDR("10.132.0.0/14")}}, + ServiceNetwork: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("172.31.0.0/16")}}, + MachineNetwork: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}}, + }, + expectConfig: true, + }, + { + name: "When the NodePool attaches the default pod network on a dual-stack cluster, it should not generate any config", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtNodePoolPlatform{ + AttachDefaultNetwork: ptr.To(true), + }, + }, + }, + }, + networking: dualStackNetworking(), + expectConfig: false, + }, + { + name: "When AttachDefaultNetwork is nil, it should default to the pod network and not generate any config", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.KubevirtPlatform, + Kubevirt: &hyperv1.KubevirtNodePoolPlatform{}, + }, + }, + }, + networking: dualStackNetworking(), + expectConfig: false, + }, + { + name: "When the NodePool platform is not KubeVirt, it should not generate any config", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.AWSPlatform, + }, + }, + }, + networking: dualStackNetworking(), + expectConfig: false, + }, + { + name: "When the KubeVirt platform spec is nil, it should not generate any config", + nodePool: &hyperv1.NodePool{ + Spec: hyperv1.NodePoolSpec{ + Platform: hyperv1.NodePoolPlatform{ + Type: hyperv1.KubevirtPlatform, + }, + }, + }, + networking: dualStackNetworking(), + expectConfig: false, + }, + { + name: "When the NodePool is nil, it should not generate any config", + nodePool: nil, + networking: dualStackNetworking(), + expectConfig: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := GenerateNetworkOverrideMachineConfig(tt.nodePool, tt.networking) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if tt.expectConfig && result == "" { + t.Fatal("expected config but got empty string") + } + if !tt.expectConfig && result != "" { + t.Fatalf("expected empty string but got config: %s", result) + } + + for _, content := range tt.expectYAMLContent { + if !strings.Contains(result, content) { + t.Errorf("expected YAML to contain %q, but it doesn't.\nConfig:\n%s", content, result) + } + } + + if len(tt.expectDecodedContent) > 0 { + decoded, err := decodeIgnitionFileContents(result) + if err != nil { + t.Fatalf("failed to decode ignition file contents: %v", err) + } + for _, content := range tt.expectDecodedContent { + if !strings.Contains(decoded, content) { + t.Errorf("expected decoded content to contain %q, but it doesn't.\nDecoded:\n%s", content, decoded) + } + } + } + }) + } +} + +func TestHasIPv6Network(t *testing.T) { + tests := []struct { + name string + networking hyperv1.ClusterNetworking + expected bool + }{ + { + name: "When all networks are IPv4, it should return false", + networking: ipv4Networking(), + expected: false, + }, + { + name: "When the cluster and service networks are dual-stack, it should return true", + networking: dualStackNetworking(), + expected: true, + }, + { + name: "When only the machine network has an IPv6 CIDR, it should return true", + networking: hyperv1.ClusterNetworking{ + MachineNetwork: []hyperv1.MachineNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd03::/64")}}, + }, + expected: true, + }, + { + name: "When only the service network has an IPv6 CIDR, it should return true", + networking: hyperv1.ClusterNetworking{ + ServiceNetwork: []hyperv1.ServiceNetworkEntry{{CIDR: *ipnet.MustParseCIDR("fd02::/112")}}, + }, + expected: true, + }, + { + name: "When networking is empty, it should return false", + networking: hyperv1.ClusterNetworking{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := hasIPv6Network(tt.networking); got != tt.expected { + t.Errorf("hasIPv6Network() = %v, expected %v", got, tt.expected) + } + }) + } +} diff --git a/test/e2e/nodepool_kv_advanced_multinet_test.go b/test/e2e/nodepool_kv_advanced_multinet_test.go index 4934ced402e5..a92af68ead2a 100644 --- a/test/e2e/nodepool_kv_advanced_multinet_test.go +++ b/test/e2e/nodepool_kv_advanced_multinet_test.go @@ -13,8 +13,10 @@ import ( . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" discoveryv1 "k8s.io/api/discovery/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/wait" @@ -34,16 +36,18 @@ import ( ) type KubeVirtAdvancedMultinetTest struct { - infra e2eutil.KubeVirtInfra - ifaceName string - nodePoolName string + infra e2eutil.KubeVirtInfra + ifaceName string + nodePoolName string + hostedClusterClient crclient.Client } -func NewKubeVirtAdvancedMultinetTest(ctx context.Context, mgmtClient crclient.Client, hc *hyperv1.HostedCluster) NodePoolTest { +func NewKubeVirtAdvancedMultinetTest(ctx context.Context, mgmtClient crclient.Client, hc *hyperv1.HostedCluster, hcClient crclient.Client) NodePoolTest { return KubeVirtAdvancedMultinetTest{ - infra: e2eutil.NewKubeVirtInfra(ctx, mgmtClient, hc), - ifaceName: "net1", - nodePoolName: hc.Name + "-" + "test-kv-advance-multinet", + infra: e2eutil.NewKubeVirtInfra(ctx, mgmtClient, hc), + ifaceName: "net1", + nodePoolName: hc.Name + "-" + "test-kv-advance-multinet", + hostedClusterClient: hcClient, } } @@ -58,7 +62,7 @@ func (k KubeVirtAdvancedMultinetTest) Setup(t *testing.T) { t.Log("Starting test KubeVirtAdvancedMultinetTest") } -func (k KubeVirtAdvancedMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePool, _ []corev1.Node) { +func (k KubeVirtAdvancedMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePool, nodes []corev1.Node) { g := NewWithT(t) np := &hyperv1.NodePool{} @@ -96,6 +100,49 @@ func (k KubeVirtAdvancedMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePoo }, }, })) + + // The HyperShift operator only neutralizes the MCO-rendered pod-network nmstate + // configuration (IPv6 autoconf disable, ARP proxy gateway) on multus-primary + // NodePools when the cluster networking includes IPv6; IPv4-only clusters are + // asymptomatic and are left untouched to avoid NodePool rollouts on operator + // upgrades. + // Capture nmstatectl output first so that a transient command failure fails the + // probe (pod stays NotReady and the test keeps waiting) instead of being treated + // as a definitive answer. + var probeCommand string + if hasIPv6HostedClusterNetwork(k.infra.HostedCluster()) { + t.Log("Verifying nmstate network configuration is NOT applied on multus primary network nodes (IPv6 cluster)") + probeCommand = `out=$(chroot /host nmstatectl show) || exit 1; ! printf '%s' "$out" | grep -q "autoconf: false"` + } else { + t.Log("Verifying MCO-rendered nmstate network configuration is still applied on multus primary network nodes (IPv4-only cluster)") + probeCommand = `chroot /host nmstatectl show | grep -q "autoconf: false"` + } + ds := composeNmstateCheckerDaemonSet(probeCommand) + dsName := "nmstate-checker-" + nodePool.Name + e2eutil.CorrelateDaemonSet(ds, &nodePool, dsName) + g.Expect(k.hostedClusterClient.Create(k.infra.Ctx(), ds)).To(Succeed()) + eventuallyDaemonSetRollsOut(t, k.infra.Ctx(), k.hostedClusterClient, len(nodes), np, ds) +} + +// hasIPv6HostedClusterNetwork returns true when any of the HostedCluster's +// cluster, service or machine networks contains an IPv6 CIDR. +func hasIPv6HostedClusterNetwork(hc *hyperv1.HostedCluster) bool { + for _, entry := range hc.Spec.Networking.ClusterNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + for _, entry := range hc.Spec.Networking.ServiceNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + for _, entry := range hc.Spec.Networking.MachineNetwork { + if net.IP(entry.CIDR.IP).To4() == nil { + return true + } + } + return false } func (k KubeVirtAdvancedMultinetTest) BuildNodePoolManifest(defaultNodepool hyperv1.NodePool) (*hyperv1.NodePool, error) { @@ -292,6 +339,80 @@ func (k KubeVirtAdvancedMultinetTest) firstMachineAddress() (string, error) { return internalAddress, nil } +// composeNmstateCheckerDaemonSet builds a privileged DaemonSet that mounts the host +// filesystem and uses a readiness probe to verify the nmstate network configuration +// via chroot /host nmstatectl show. The probeCommand should be a shell command that +// returns 0 when the expected network state is found. +func composeNmstateCheckerDaemonSet(probeCommand string) *appsv1.DaemonSet { + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nmstate-checker", + Namespace: "kube-system", + }, + Spec: appsv1.DaemonSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "name": "nmstate-checker", + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "name": "nmstate-checker", + }, + }, + Spec: corev1.PodSpec{ + HostPID: true, + Tolerations: []corev1.Toleration{ + {Operator: corev1.TolerationOpExists}, + }, + Containers: []corev1.Container{ + { + Name: "nmstate-checker", + Image: "registry.access.redhat.com/ubi9/ubi:latest", + Command: []string{"/bin/sleep", "24h"}, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("200Mi"), + }, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + Exec: &corev1.ExecAction{ + Command: []string{"/bin/sh", "-c", probeCommand}, + }, + }, + }, + SecurityContext: &corev1.SecurityContext{ + Privileged: ptr.To(true), + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: "host", + MountPath: "/host", + ReadOnly: true, + }, + }, + }, + }, + TerminationGracePeriodSeconds: ptr.To[int64](30), + Volumes: []corev1.Volume{ + { + Name: "host", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/", + }, + }, + }, + }, + }, + }, + }, + } +} + func (k KubeVirtAdvancedMultinetTest) composeDNSMasqPod(t *testing.T) *corev1.Pod { g := NewWithT(t) podName := k.infra.NADName() + "-dnsmasq" diff --git a/test/e2e/nodepool_kv_multinet_test.go b/test/e2e/nodepool_kv_multinet_test.go index 658e724ba16e..9a56ed613dfd 100644 --- a/test/e2e/nodepool_kv_multinet_test.go +++ b/test/e2e/nodepool_kv_multinet_test.go @@ -22,12 +22,14 @@ import ( ) type KubeVirtMultinetTest struct { - infra e2eutil.KubeVirtInfra + infra e2eutil.KubeVirtInfra + hostedClusterClient crclient.Client } -func NewKubeVirtMultinetTest(ctx context.Context, cl crclient.Client, hc *hyperv1.HostedCluster) *KubeVirtMultinetTest { +func NewKubeVirtMultinetTest(ctx context.Context, cl crclient.Client, hc *hyperv1.HostedCluster, hcClient crclient.Client) *KubeVirtMultinetTest { return &KubeVirtMultinetTest{ - infra: e2eutil.NewKubeVirtInfra(ctx, cl, hc), + infra: e2eutil.NewKubeVirtInfra(ctx, cl, hc), + hostedClusterClient: hcClient, } } @@ -39,7 +41,7 @@ func (k KubeVirtMultinetTest) Setup(t *testing.T) { t.Log("Starting test KubeVirtMultinetTest") } -func (k KubeVirtMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePool, _ []corev1.Node) { +func (k KubeVirtMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePool, nodes []corev1.Node) { g := NewWithT(t) np := &hyperv1.NodePool{} @@ -108,6 +110,16 @@ func (k KubeVirtMultinetTest) Run(t *testing.T, nodePool hyperv1.NodePool, _ []c }, }, ) + + // Verify nmstate network config IS applied when using the default pod network. + // When the default network is attached, the pod-network-specific nmstate configuration + // (IPv6 autoconf disable, ARP proxy gateway) should be present on the nodes. + t.Log("Verifying nmstate network configuration IS applied on default pod network nodes") + ds := composeNmstateCheckerDaemonSet(`chroot /host nmstatectl show | grep -q "autoconf: false"`) + dsName := "nmstate-checker-" + nodePool.Name + e2eutil.CorrelateDaemonSet(ds, &nodePool, dsName) + g.Expect(k.hostedClusterClient.Create(k.infra.Ctx(), ds)).To(Succeed()) + eventuallyDaemonSetRollsOut(t, k.infra.Ctx(), k.hostedClusterClient, len(nodes), np, ds) } func (k KubeVirtMultinetTest) BuildNodePoolManifest(defaultNodepool hyperv1.NodePool) (*hyperv1.NodePool, error) { diff --git a/test/e2e/nodepool_test.go b/test/e2e/nodepool_test.go index 7e212ce72866..c6d2abb06bac 100644 --- a/test/e2e/nodepool_test.go +++ b/test/e2e/nodepool_test.go @@ -121,7 +121,7 @@ func TestNodePool(t *testing.T) { }, { name: "KubeVirtNodeMultinetTest", - test: NewKubeVirtMultinetTest(ctx, mgtClient, hostedCluster), + test: NewKubeVirtMultinetTest(ctx, mgtClient, hostedCluster, hostedClusterClient), }, { name: "OpenStackAdvancedTest", @@ -181,7 +181,7 @@ func TestNodePool(t *testing.T) { return []NodePoolTestCase{ { name: "KubeVirtNodeAdvancedMultinetTest", - test: NewKubeVirtAdvancedMultinetTest(ctx, mgtClient, hostedCluster), + test: NewKubeVirtAdvancedMultinetTest(ctx, mgtClient, hostedCluster, hostedClusterClient), }, { name: "KubeVirtHostNetworkIngressPassthroughTest", @@ -247,11 +247,14 @@ func executeNodePoolTests(t *testing.T, nodePoolTestCasesPerHostedCluster []Host clusterOpts.AWSPlatform.SharedRole = false } - // On OpenStack, we need to create at least one replica of the default nodepool - // so we can create the Route53 record for the ingress router. If we don't do that, - // the HostedCluster conditions won't be met and the test will fail as some operators - // will be marked as degraded. - if globalOpts.Platform == hyperv1.OpenStackPlatform { + // On OpenStack and KubeVirt, we need to create at least one replica of the default nodepool. + // On OpenStack, this is needed to create the Route53 record for the ingress router. + // On KubeVirt, CNO requires worker nodes to probe the network MTU before it can + // deploy its operands (ovnkube-control-plane, network-node-identity, multus-admission-controller), + // without which controlPlaneVersion never reaches Completed. + // If we don't do that, the HostedCluster conditions won't be met and the test will + // fail as some operators will be marked as degraded. + if globalOpts.Platform == hyperv1.OpenStackPlatform || globalOpts.Platform == hyperv1.KubevirtPlatform { clusterOpts.NodePoolReplicas = 1 }