diff --git a/pkg/render/common/components/components.go b/pkg/render/common/components/components.go index eaa3553c1d..e053cb0089 100644 --- a/pkg/render/common/components/components.go +++ b/pkg/render/common/components/components.go @@ -275,6 +275,21 @@ func GetNodeSelector(overrides any) map[string]string { return value.Interface().(map[string]string) } +// GetResources returns the pod-level Spec.Template.Spec.Resources override, or nil +// if unset. The shared override path doesn't apply it, because only the +// component knows which container should receive it. +func GetResources(overrides any) *corev1.ResourceRequirements { + value := getField(overrides, "Spec", "Template", "Spec", "Resources") + if !value.IsValid() || value.IsNil() { + return nil + } + r, ok := value.Interface().(*corev1.ResourceRequirements) + if !ok { + return nil + } + return r +} + func GetTopologySpreadConstraints(overrides any) []corev1.TopologySpreadConstraint { value := getField(overrides, "Spec", "Template", "Spec", "TopologySpreadConstraints") if !value.IsValid() || value.IsNil() { diff --git a/pkg/render/istio/istio.go b/pkg/render/istio/istio.go index 98d2fb9141..8de39d89ef 100644 --- a/pkg/render/istio/istio.go +++ b/pkg/render/istio/istio.go @@ -78,6 +78,10 @@ const ( IstioIstiodServiceName = "istiod" istioFakeImageProxyv2 = "fake.io/fakeimg/proxyv2:faketag" + + istiodContainerName = "discovery" + istioCNIContainerName = "install-cni" + ztunnelContainerName = "istio-proxy" ) func Istio(cfg *Configuration) (*IstioComponentCRDs, *IstioComponent, error) { @@ -171,19 +175,19 @@ func (c *IstioComponent) patchImages() (err error) { for i := range c.resources.IstiodDeployment.Spec.Template.Spec.Containers { container := &c.resources.IstiodDeployment.Spec.Template.Spec.Containers[i] - if container.Name == "discovery" { + if container.Name == istiodContainerName { container.Image = c.IstioPilotImage } } for i := range c.resources.CNIDaemonSet.Spec.Template.Spec.Containers { container := &c.resources.CNIDaemonSet.Spec.Template.Spec.Containers[i] - if container.Name == "install-cni" { + if container.Name == istioCNIContainerName { container.Image = c.IstioInstallCNIImage } } for i := range c.resources.ZTunnelDaemonSet.Spec.Template.Spec.Containers { container := &c.resources.ZTunnelDaemonSet.Spec.Template.Spec.Containers[i] - if container.Name == "istio-proxy" { + if container.Name == ztunnelContainerName { container.Image = c.IstioZTunnelImage } } @@ -273,6 +277,12 @@ func (c *IstioComponent) Objects() ([]client.Object, []client.Object) { networkpolicy.DeprecatedAllowTigeraNetworkPolicyObject("ztunnel", IstioNamespace), ) + // The Istio CR's resources field sits at pod level, so the shared override + // path can't place it. Each workload has one main container to receive it. + setContainerResources(&res.IstiodDeployment.Spec.Template.Spec, istiodContainerName, rcomp.GetResources(c.cfg.Istio.Spec.IstiodDeployment)) + setContainerResources(&res.CNIDaemonSet.Spec.Template.Spec, istioCNIContainerName, rcomp.GetResources(c.cfg.Istio.Spec.IstioCNIDaemonset)) + setContainerResources(&res.ZTunnelDaemonSet.Spec.Template.Spec, ztunnelContainerName, rcomp.GetResources(c.cfg.Istio.Spec.ZTunnelDaemonset)) + if overrides := c.cfg.Istio.Spec.IstiodDeployment; overrides != nil { rcomp.ApplyDeploymentOverrides(res.IstiodDeployment, overrides) } @@ -288,7 +298,7 @@ func (c *IstioComponent) Objects() ([]client.Object, []client.Object) { // Set required configs for i := range res.ZTunnelDaemonSet.Spec.Template.Spec.Containers { cont := &res.ZTunnelDaemonSet.Spec.Template.Spec.Containers[i] - if cont.Name == "istio-proxy" { + if cont.Name == ztunnelContainerName { cont.Env = append(cont.Env, corev1.EnvVar{ Name: "TRANSPARENT_NETWORK_POLICIES", Value: "true", @@ -298,7 +308,7 @@ func (c *IstioComponent) Objects() ([]client.Object, []client.Object) { } for i := range res.CNIDaemonSet.Spec.Template.Spec.Containers { cont := &res.CNIDaemonSet.Spec.Template.Spec.Containers[i] - if cont.Name == "install-cni" { + if cont.Name == istioCNIContainerName { cont.Env = append(cont.Env, corev1.EnvVar{ Name: "MAGIC_DSCP_MARK", Value: strconv.FormatInt(int64(c.cfg.Istio.Spec.DSCPMark.ToUint8()), 10), @@ -468,3 +478,12 @@ func (c *IstioComponentCRDs) ResolveImages(is *operatorv1.ImageSet) error { func (c *IstioComponentCRDs) SupportedOSType() rmeta.OSType { return rmeta.OSTypeLinux } + +// setContainerResources replaces the resources of the named container with r. +// A nil r leaves the Helm chart's defaults in place. +func setContainerResources(spec *corev1.PodSpec, name string, r *corev1.ResourceRequirements) { + if r == nil { + return + } + render.MustContainer(spec, name).Resources = *r +} diff --git a/pkg/render/istio/istio_test.go b/pkg/render/istio/istio_test.go index 50921ccea7..38bd78fa33 100644 --- a/pkg/render/istio/istio_test.go +++ b/pkg/render/istio/istio_test.go @@ -531,20 +531,27 @@ var _ = Describe("Istio Component Rendering", func() { }, }, func(deployment *appsv1.Deployment) { - // Resources are applied to the deployment - // The override system applies resources to the pod spec, not individual containers - // So we just verify that the deployment has at least one container with resources set - hasResourcesSet := false - for _, container := range deployment.Spec.Template.Spec.Containers { - if container.Resources.Limits != nil || container.Resources.Requests != nil { - hasResourcesSet = true - break - } - } - Expect(hasResourcesSet).To(BeTrue(), "Expected at least one container to have resources set") + Expect(deployment.Spec.Template.Spec.Containers).To(ContainElement(SatisfyAll( + HaveField("Name", "discovery"), + HaveField("Resources", corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"memory": resource.MustParse("2Gi")}, + Requests: corev1.ResourceList{"memory": resource.MustParse("1Gi")}, + }), + ))) }, ), ) + + It("should keep the chart's default resources when no override is set", func() { + _, component, err := istio.Istio(cfg) + Expect(err).ShouldNot(HaveOccurred()) + + objsToCreate, _ := component.Objects() + deployment, err := rtest.GetResourceOfType[*appsv1.Deployment](objsToCreate, istio.IstioIstiodDeploymentName, istio.IstioNamespace) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(deployment.Spec.Template.Spec.Containers).To(ContainElement(SatisfyAll(HaveField("Name", "discovery"), HaveField("Resources.Requests", Not(BeEmpty()))))) + }) }) Describe("DaemonSet Overrides", func() { @@ -599,6 +606,29 @@ var _ = Describe("Istio Component Rendering", func() { })) }, ), + Entry("should apply resources override", + &operatorv1.IstioCNIDaemonset{ + Spec: &operatorv1.IstioCNIDaemonsetSpec{ + Template: &operatorv1.IstioCNIDaemonsetSpecTemplate{ + Spec: &operatorv1.IstioCNIDaemonsetPodSpec{ + Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"cpu": resource.MustParse("500m")}, + Requests: corev1.ResourceList{"cpu": resource.MustParse("200m")}, + }, + }, + }, + }, + }, + func(daemonset *appsv1.DaemonSet) { + Expect(daemonset.Spec.Template.Spec.Containers).To(ContainElement(SatisfyAll( + HaveField("Name", "install-cni"), + HaveField("Resources", corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"cpu": resource.MustParse("500m")}, + Requests: corev1.ResourceList{"cpu": resource.MustParse("200m")}, + }), + ))) + }, + ), ) DescribeTable("ZTunnel DaemonSet Overrides", func(overrides *operatorv1.ZTunnelDaemonset, verify func(*appsv1.DaemonSet)) { @@ -659,6 +689,29 @@ var _ = Describe("Istio Component Rendering", func() { Expect(daemonset.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions[0].Key).To(Equal("ztunnel-affinity-key")) }, ), + Entry("should apply resources override", + &operatorv1.ZTunnelDaemonset{ + Spec: &operatorv1.ZTunnelDaemonsetSpec{ + Template: &operatorv1.ZTunnelDaemonsetSpecTemplate{ + Spec: &operatorv1.ZTunnelDaemonsetPodSpec{ + Resources: &corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"memory": resource.MustParse("3Gi")}, + Requests: corev1.ResourceList{"memory": resource.MustParse("512Mi")}, + }, + }, + }, + }, + }, + func(daemonset *appsv1.DaemonSet) { + Expect(daemonset.Spec.Template.Spec.Containers).To(ContainElement(SatisfyAll( + HaveField("Name", "istio-proxy"), + HaveField("Resources", corev1.ResourceRequirements{ + Limits: corev1.ResourceList{"memory": resource.MustParse("3Gi")}, + Requests: corev1.ResourceList{"memory": resource.MustParse("512Mi")}, + }), + ))) + }, + ), ) }) diff --git a/pkg/render/node.go b/pkg/render/node.go index 669abd6b17..4cab1983f5 100644 --- a/pkg/render/node.go +++ b/pkg/render/node.go @@ -646,7 +646,7 @@ func (c *nodeComponent) createCalicoPluginConfig() map[string]any { ipam := c.getCalicoIPAM() if c.cfg.Installation.CNI.IPAM.Type == operatorv1.IPAMPluginHostLocal { - ipam = buildHostLocalIPAM(c.cfg.IPPools) + ipam = buildHostLocalIPAM(WorkloadIPPools(c.cfg.IPPools)) } apiRoot := c.cfg.K8sServiceEp.CNIAPIRoot() @@ -805,14 +805,15 @@ func (c *nodeComponent) nodeCNIConfigMap() *corev1.ConfigMap { func (c *nodeComponent) getCalicoIPAM() map[string]any { // Determine what address families to enable. + pools := WorkloadIPPools(c.cfg.IPPools) var assign_ipv4 string var assign_ipv6 string - if v4pool := GetIPv4Pool(c.cfg.IPPools); v4pool != nil { + if v4pool := GetIPv4Pool(pools); v4pool != nil { assign_ipv4 = "true" } else { assign_ipv4 = "false" } - if v6pool := GetIPv6Pool(c.cfg.IPPools); v6pool != nil { + if v6pool := GetIPv6Pool(pools); v6pool != nil { assign_ipv6 = "true" } else { assign_ipv6 = "false" @@ -1794,6 +1795,31 @@ func GetIPv6Pool(pools []operatorv1.IPPool) *operatorv1.IPPool { return nil } +// WorkloadIPPools returns the pools that can hand out addresses to ordinary workloads. +func WorkloadIPPools(pools []operatorv1.IPPool) []operatorv1.IPPool { + filtered := []operatorv1.IPPool{} + for _, pool := range pools { + if allowsWorkloads(pool) { + filtered = append(filtered, pool) + } + } + return filtered +} + +func allowsWorkloads(pool operatorv1.IPPool) bool { + if len(pool.AllowedUses) == 0 { + // An unset allowedUses means Workload and Tunnel, for back-compatibility. + return true + } + + for _, use := range pool.AllowedUses { + if use == operatorv1.IPPoolAllowedUseWorkload { + return true + } + } + return false +} + // bgpEnabled returns true if the given Installation enables BGP, false otherwise. func bgpEnabled(instance *operatorv1.InstallationSpec) bool { return instance.CalicoNetwork != nil && diff --git a/pkg/render/node_test.go b/pkg/render/node_test.go index 0b1888f9c1..d990305ecf 100644 --- a/pkg/render/node_test.go +++ b/pkg/render/node_test.go @@ -15,6 +15,7 @@ package render_test import ( + "encoding/json" "fmt" "strings" @@ -3271,3 +3272,126 @@ func verifyInitContainers(ds *appsv1.DaemonSet, instance *operatorv1.Installatio Expect(flexvolContainer).To(BeNil()) } } + +var _ = Describe("Node CNI address family selection", func() { + const l2Workload = operatorv1.IPPoolAllowedUse("L2Workload") + + var cfg render.NodeConfiguration + var instance *operatorv1.InstallationSpec + + // ipamConfig renders the CNI config map and returns the ipam block of the calico plugin. + ipamConfig := func() map[string]any { + component := render.Node(&cfg) + Expect(component.ResolveImages(nil)).To(BeNil()) + resources, _ := component.Objects() + + cm, ok := rtest.GetResource(resources, "cni-config", "calico-system", "", "v1", "ConfigMap").(*corev1.ConfigMap) + Expect(ok).To(BeTrue()) + + var parsed cniNetworkConfig + Expect(json.Unmarshal([]byte(cm.Data["config"]), &parsed)).NotTo(HaveOccurred()) + Expect(parsed.Plugins).NotTo(BeEmpty()) + return parsed.Plugins[0].IPAM + } + + BeforeEach(func() { + ff := true + instance = &operatorv1.InstallationSpec{ + CNI: &operatorv1.CNISpec{ + Type: operatorv1.PluginCalico, + IPAM: &operatorv1.IPAMSpec{Type: operatorv1.IPAMPluginCalico}, + }, + CalicoNetwork: &operatorv1.CalicoNetworkSpec{ + BGP: &bgpEnabled, + IPPools: []operatorv1.IPPool{{CIDR: "192.168.0.0/16"}}, + NodeAddressAutodetectionV4: &operatorv1.NodeAddressAutodetection{FirstFound: &ff}, + }, + } + defaultCNIConfDir, defaultCNIBinDir := render.DefaultCNIDirectories(instance.KubernetesProvider) + instance.CNI.ConfDir, instance.CNI.BinDir = &defaultCNIConfDir, &defaultCNIBinDir + + scheme := runtime.NewScheme() + Expect(apis.AddToScheme(scheme, false)).NotTo(HaveOccurred()) + cli := ctrlrfake.DefaultFakeClientBuilder(scheme).Build() + certificateManager, err := certificatemanager.Create(cli, nil, clusterDomain, common.OperatorNamespace(), certificatemanager.AllowCACreation()) + Expect(err).NotTo(HaveOccurred()) + + cfg = render.NodeConfiguration{ + K8sServiceEp: k8sapi.ServiceEndpoint{}, + Installation: instance, + TLS: getTyphaNodeTLS(cli, certificateManager), + ClusterDomain: clusterDomain, + FelixHealthPort: 9099, + IPPools: instance.CalicoNetwork.IPPools, + ImageOverrides: imageoverride.New(), + } + }) + + It("should not enable IPv6 assignment for a pool that doesn't allow workloads", func() { + cfg.IPPools = append(cfg.IPPools, operatorv1.IPPool{ + CIDR: "2001:db8:1::/122", + AllowedUses: []operatorv1.IPPoolAllowedUse{l2Workload}, + }) + ipam := ipamConfig() + Expect(ipam).To(HaveKeyWithValue("assign_ipv4", "true")) + Expect(ipam).To(HaveKeyWithValue("assign_ipv6", "false")) + }) + + It("should not enable IPv6 assignment for a tunnel-only IPv6 pool", func() { + cfg.IPPools = append(cfg.IPPools, operatorv1.IPPool{ + CIDR: "2001:db8:1::/122", + AllowedUses: []operatorv1.IPPoolAllowedUse{operatorv1.IPPoolAllowedUseTunnel}, + }) + Expect(ipamConfig()).To(HaveKeyWithValue("assign_ipv6", "false")) + }) + + It("should enable IPv6 assignment for a pool that allows workloads", func() { + cfg.IPPools = append(cfg.IPPools, operatorv1.IPPool{ + CIDR: "2001:db8:1::/122", + AllowedUses: []operatorv1.IPPoolAllowedUse{l2Workload, operatorv1.IPPoolAllowedUseWorkload}, + }) + Expect(ipamConfig()).To(HaveKeyWithValue("assign_ipv6", "true")) + }) + + It("should enable IPv6 assignment for a pool with no allowed uses set", func() { + cfg.IPPools = append(cfg.IPPools, operatorv1.IPPool{CIDR: "2001:db8:1::/122"}) + Expect(ipamConfig()).To(HaveKeyWithValue("assign_ipv6", "true")) + }) + + It("should enable IPv6 assignment when a later IPv6 pool allows workloads", func() { + cfg.IPPools = append(cfg.IPPools, + operatorv1.IPPool{ + CIDR: "2001:db8:1::/122", + AllowedUses: []operatorv1.IPPoolAllowedUse{operatorv1.IPPoolAllowedUseLoadBalancer}, + }, + operatorv1.IPPool{CIDR: "2001:db8:2::/122"}, + ) + Expect(ipamConfig()).To(HaveKeyWithValue("assign_ipv6", "true")) + }) + + It("should disable IPv4 assignment when the only IPv4 pool is tunnel-only", func() { + cfg.IPPools = []operatorv1.IPPool{{ + CIDR: "192.168.0.0/16", + AllowedUses: []operatorv1.IPPoolAllowedUse{operatorv1.IPPoolAllowedUseTunnel}, + }} + Expect(ipamConfig()).To(HaveKeyWithValue("assign_ipv4", "false")) + }) + + It("should render host-local as single-stack when the IPv6 pool doesn't allow workloads", func() { + instance.CNI.IPAM.Type = operatorv1.IPAMPluginHostLocal + cfg.IPPools = append(cfg.IPPools, operatorv1.IPPool{ + CIDR: "2001:db8:1::/122", + AllowedUses: []operatorv1.IPPoolAllowedUse{l2Workload}, + }) + Expect(ipamConfig()).To(Equal(map[string]any{"type": "host-local", "subnet": "usePodCidr"})) + }) +}) + +// cniNetworkConfig mirrors the parts of the rendered CNI network config the tests assert on. +type cniNetworkConfig struct { + Plugins []cniPluginConfig `json:"plugins"` +} + +type cniPluginConfig struct { + IPAM map[string]any `json:"ipam"` +}