From 8eaa1309e2b084356ec21b9ecf51d99a8caa92b2 Mon Sep 17 00:00:00 2001 From: Michal Fupso Date: Wed, 11 Sep 2024 12:01:14 -0700 Subject: [PATCH 1/2] Update operator to enable LoadBalancer kube-controller --- api/v1/installation_types.go | 12 +++++- pkg/apis/crd.projectcalico.org/v1/ippool.go | 15 +++++++- pkg/controller/ippool/defaults.go | 4 ++ pkg/controller/ippool/pool_controller_test.go | 3 ++ pkg/controller/ippool/validation.go | 38 +++++++++++++++++-- pkg/controller/migration/convert/ippools.go | 1 + .../calico/crd.projectcalico.org_ippools.yaml | 5 +++ ...ico.org_kubecontrollersconfigurations.yaml | 14 +++++++ .../operator.tigera.io_installations.yaml | 10 +++++ .../kubecontrollers/kube-controllers.go | 9 ++++- .../kubecontrollers/kube-controllers_test.go | 12 +++--- 11 files changed, 108 insertions(+), 15 deletions(-) diff --git a/api/v1/installation_types.go b/api/v1/installation_types.go index 41287db102..6e0dd6a773 100644 --- a/api/v1/installation_types.go +++ b/api/v1/installation_types.go @@ -673,13 +673,17 @@ type IPPool struct { // AllowedUse controls what the IP pool will be used for. If not specified or empty, defaults to // ["Tunnel", "Workload"] for back-compatibility AllowedUses []IPPoolAllowedUse `json:"allowedUses,omitempty" validate:"omitempty"` + + // AssignmentMode determines if IP addresses from this pool should be assigned automatically or on request only + AssignmentMode pcv1.AssignmentMode `json:"assignmentMode,omitempty" validate:"omitempty,assignmentMode"` } type IPPoolAllowedUse string const ( - IPPoolAllowedUseWorkload IPPoolAllowedUse = "Workload" - IPPoolAllowedUseTunnel IPPoolAllowedUse = "Tunnel" + IPPoolAllowedUseWorkload IPPoolAllowedUse = "Workload" + IPPoolAllowedUseTunnel IPPoolAllowedUse = "Tunnel" + IPPoolAllowedUseLoadBalancer IPPoolAllowedUse = "LoadBalancer" ) // ToProjectCalicoV1 converts an IPPool to a crd.projectcalico.org/v1 IPPool resource. @@ -736,6 +740,8 @@ func (p *IPPool) ToProjectCalicoV1() (*pcv1.IPPool, error) { pool.Spec.AllowedUses = append(pool.Spec.AllowedUses, pcv1.IPPoolAllowedUse(use)) } + pool.Spec.AssignmentMode = p.AssignmentMode + return &pool, nil } @@ -791,6 +797,8 @@ func (p *IPPool) FromProjectCalicoV1(crd pcv1.IPPool) { for _, use := range crd.Spec.AllowedUses { p.AllowedUses = append(p.AllowedUses, IPPoolAllowedUse(use)) } + + p.AssignmentMode = crd.Spec.AssignmentMode } // CNIPluginType describes the type of CNI plugin used. diff --git a/pkg/apis/crd.projectcalico.org/v1/ippool.go b/pkg/apis/crd.projectcalico.org/v1/ippool.go index c6ffc3ab6a..07324a8a34 100644 --- a/pkg/apis/crd.projectcalico.org/v1/ippool.go +++ b/pkg/apis/crd.projectcalico.org/v1/ippool.go @@ -74,13 +74,17 @@ type IPPoolSpec struct { // AllowedUse controls what the IP pool will be used for. If not specified or empty, defaults to // ["Tunnel", "Workload"] for back-compatibility AllowedUses []IPPoolAllowedUse `json:"allowedUses,omitempty" validate:"omitempty"` + + // AssignmentMode determines if IP addresses from this pool should be assigned automatically or on request only + AssignmentMode AssignmentMode `json:"assignmentMode,omitempty" validate:"omitempty,assignmentMode"` } type IPPoolAllowedUse string const ( - IPPoolAllowedUseWorkload IPPoolAllowedUse = "Workload" - IPPoolAllowedUseTunnel IPPoolAllowedUse = "Tunnel" + IPPoolAllowedUseWorkload IPPoolAllowedUse = "Workload" + IPPoolAllowedUseTunnel IPPoolAllowedUse = "Tunnel" + IPPoolsAllowedUseLoadBalancer IPPoolAllowedUse = "LoadBalancer" ) type VXLANMode string @@ -99,6 +103,13 @@ const ( IPIPModeCrossSubnet IPIPMode = "CrossSubnet" ) +type AssignmentMode string + +const ( + Automatic AssignmentMode = "Automatic" + Manual AssignmentMode = "Manual" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // IPPoolList contains a list of IPPool resources. diff --git a/pkg/controller/ippool/defaults.go b/pkg/controller/ippool/defaults.go index a775b2dab8..ccc37fb7e7 100644 --- a/pkg/controller/ippool/defaults.go +++ b/pkg/controller/ippool/defaults.go @@ -235,6 +235,10 @@ func fillDefaults(ctx context.Context, client client.Client, instance *operator. } } } + + if pool.AssignmentMode == "" { + pool.AssignmentMode = crdv1.Automatic + } } return nil } diff --git a/pkg/controller/ippool/pool_controller_test.go b/pkg/controller/ippool/pool_controller_test.go index 59625ca530..e30c13fa38 100644 --- a/pkg/controller/ippool/pool_controller_test.go +++ b/pkg/controller/ippool/pool_controller_test.go @@ -452,6 +452,7 @@ var _ = table.DescribeTable("Test OpenShift IP pool defaulting", BlockSize: &twentySix, AllowedUses: []operator.IPPoolAllowedUse{operator.IPPoolAllowedUseWorkload, operator.IPPoolAllowedUseTunnel}, DisableNewAllocations: &false_, + AssignmentMode: "Automatic", }, }, }), @@ -481,6 +482,7 @@ var _ = table.DescribeTable("Test OpenShift IP pool defaulting", BlockSize: &twentySix, AllowedUses: []operator.IPPoolAllowedUse{operator.IPPoolAllowedUseWorkload, operator.IPPoolAllowedUseTunnel}, DisableNewAllocations: &false_, + AssignmentMode: "Automatic", }, }, }), @@ -518,6 +520,7 @@ var _ = table.DescribeTable("Test OpenShift IP pool defaulting", BlockSize: &twentySix, AllowedUses: []operator.IPPoolAllowedUse{operator.IPPoolAllowedUseWorkload, operator.IPPoolAllowedUseTunnel}, DisableNewAllocations: &false_, + AssignmentMode: "Automatic", }, }, }), diff --git a/pkg/controller/ippool/validation.go b/pkg/controller/ippool/validation.go index 961a4aded9..75984b357d 100644 --- a/pkg/controller/ippool/validation.go +++ b/pkg/controller/ippool/validation.go @@ -42,9 +42,30 @@ func ValidatePools(instance *operator.Installation) error { } names[pool.Name] = true + // Check if pool is for LoadBalancer + isLoadBalancer := false + for _, u := range pool.AllowedUses { + if u == operator.IPPoolAllowedUseLoadBalancer { + isLoadBalancer = true + } + } + + // Check if pool is set as LoadBalancer no other allowed use is specified + if isLoadBalancer { + for _, u := range pool.AllowedUses { + if u != operator.IPPoolAllowedUseLoadBalancer { + return fmt.Errorf("IP pool %s AllowedUse LoadBalancer cannot be used with Workload/Tunnel", pool.Name) + } + } + } + // Verify NAT outgoing values. switch pool.NATOutgoing { - case operator.NATOutgoingEnabled, operator.NATOutgoingDisabled: + case operator.NATOutgoingEnabled: + case operator.NATOutgoingDisabled: + if isLoadBalancer { + return fmt.Errorf("IP pool %s NATOutgoing cannot be disabled with allowedUse LoadBalancer", pool.Name) + } default: return fmt.Errorf("%s is invalid for natOutgoing, should be one of %s", pool.NATOutgoing, strings.Join(operator.NATOutgoingTypesString, ",")) @@ -54,6 +75,11 @@ func ValidatePools(instance *operator.Installation) error { if pool.NodeSelector == "" { return fmt.Errorf("IP pool nodeSelector should not be empty") } + + if isLoadBalancer && pool.NodeSelector != "all()" { + return fmt.Errorf("IP pool nodeSelector should be set to all() if allowedUse is LoadBalancer") + } + if instance.Spec.CNI == nil { // We expect this to be defaulted by the core Installation controller prior to the IP pool controller // being invoked, but check just in case. @@ -67,8 +93,10 @@ func ValidatePools(instance *operator.Installation) error { // Verify the Encapsulation mode is valid. switch pool.Encapsulation { - case operator.EncapsulationIPIP, operator.EncapsulationIPIPCrossSubnet: - case operator.EncapsulationVXLAN, operator.EncapsulationVXLANCrossSubnet: + case operator.EncapsulationIPIP, operator.EncapsulationIPIPCrossSubnet, operator.EncapsulationVXLAN, operator.EncapsulationVXLANCrossSubnet: + if isLoadBalancer { + return fmt.Errorf("IP pool encapsulation must be none if allowedUse is LoadBalancer") + } case operator.EncapsulationNone: default: return fmt.Errorf("%s is invalid for ipPool.encapsulation, should be one of %s", @@ -104,6 +132,10 @@ func ValidatePools(instance *operator.Installation) error { } } } + + if isLoadBalancer && *pool.DisableBGPExport { + return fmt.Errorf("IP pool disable bgp export must be false when AllowedUse is LoadBalancer") + } } return nil } diff --git a/pkg/controller/migration/convert/ippools.go b/pkg/controller/migration/convert/ippools.go index 11d1bc6fc3..2ddb331a01 100644 --- a/pkg/controller/migration/convert/ippools.go +++ b/pkg/controller/migration/convert/ippools.go @@ -263,6 +263,7 @@ func convertPool(src crdv1.IPPool) (operatorv1.IPPool, error) { p.NodeSelector = src.Spec.NodeSelector p.DisableBGPExport = &src.Spec.DisableBGPExport + p.AssignmentMode = src.Spec.AssignmentMode return p, nil } diff --git a/pkg/crds/calico/crd.projectcalico.org_ippools.yaml b/pkg/crds/calico/crd.projectcalico.org_ippools.yaml index 83311f963a..c63c921c82 100644 --- a/pkg/crds/calico/crd.projectcalico.org_ippools.yaml +++ b/pkg/crds/calico/crd.projectcalico.org_ippools.yaml @@ -94,6 +94,11 @@ spec: If not specified, then this is defaulted to "Never" (i.e. VXLAN tunneling is disabled). type: string + assignmentMode: + description: AssignmentMode determines if IP addresses + from this pool should be assigned automatically or + on request only + type: string required: - cidr type: object diff --git a/pkg/crds/calico/crd.projectcalico.org_kubecontrollersconfigurations.yaml b/pkg/crds/calico/crd.projectcalico.org_kubecontrollersconfigurations.yaml index 504de3e39f..cebd2f6b57 100644 --- a/pkg/crds/calico/crd.projectcalico.org_kubecontrollersconfigurations.yaml +++ b/pkg/crds/calico/crd.projectcalico.org_kubecontrollersconfigurations.yaml @@ -99,6 +99,13 @@ spec: with the Calico datastore. [Default: 5m]' type: string type: object + loadBalancer: + description: LoadBalancer enables and configures the LoadBalancer + controller. Enabled by default, set to nil to disable. + properties: + assignIPs: + type: string + type: object type: object debugProfilePort: description: DebugProfilePort configures the port to serve memory @@ -214,6 +221,13 @@ spec: 5m]' type: string type: object + loadBalancer: + description: LoadBalancer enables and configures the LoadBalancer + controller. Enabled by default, set to nil to disable. + properties: + assignIPs: + type: string + type: object type: object debugProfilePort: description: DebugProfilePort configures the port to serve memory diff --git a/pkg/crds/operator/operator.tigera.io_installations.yaml b/pkg/crds/operator/operator.tigera.io_installations.yaml index 941e8940b4..979a4bdcf4 100644 --- a/pkg/crds/operator/operator.tigera.io_installations.yaml +++ b/pkg/crds/operator/operator.tigera.io_installations.yaml @@ -1083,6 +1083,11 @@ spec: items: type: string type: array + assignmentMode: + description: AssignmentMode determines if IP addresses from + this pool should be assigned automatically or on request + only + type: string blockSize: description: |- BlockSize specifies the CIDR prefex length to use when allocating per-node IP blocks from @@ -8467,6 +8472,11 @@ spec: items: type: string type: array + assignmentMode: + description: AssignmentMode determines if IP addresses + from this pool should be assigned automatically or + on request only + type: string blockSize: description: |- BlockSize specifies the CIDR prefex length to use when allocating per-node IP blocks from diff --git a/pkg/render/kubecontrollers/kube-controllers.go b/pkg/render/kubecontrollers/kube-controllers.go index 2ddee05d0a..3c655b07f5 100644 --- a/pkg/render/kubecontrollers/kube-controllers.go +++ b/pkg/render/kubecontrollers/kube-controllers.go @@ -107,7 +107,7 @@ type KubeControllersConfiguration struct { func NewCalicoKubeControllers(cfg *KubeControllersConfiguration) *kubeControllersComponent { kubeControllerRolePolicyRules := kubeControllersRoleCommonRules(cfg, KubeController) - enabledControllers := []string{"node"} + enabledControllers := []string{"node", "loadbalancer"} if cfg.Installation.Variant == operatorv1.TigeraSecureEnterprise { kubeControllerRolePolicyRules = append(kubeControllerRolePolicyRules, kubeControllersRoleEnterpriseCommonRules(cfg)...) kubeControllerRolePolicyRules = append(kubeControllerRolePolicyRules, @@ -365,6 +365,11 @@ func kubeControllersRoleCommonRules(cfg *KubeControllersConfiguration, kubeContr Resources: []string{"pods"}, Verbs: []string{"get", "list", "watch"}, }, + { + APIGroups: []string{""}, + Resources: []string{"services", "services/status"}, + Verbs: []string{"get", "list", "update", "watch"}, + }, { // IPAM resources are manipulated in response to node and block updates, as well as periodic triggers. APIGroups: []string{"crd.projectcalico.org"}, @@ -373,7 +378,7 @@ func kubeControllersRoleCommonRules(cfg *KubeControllersConfiguration, kubeContr }, { APIGroups: []string{"crd.projectcalico.org"}, - Resources: []string{"blockaffinities", "ipamblocks", "ipamhandles", "networksets"}, + Resources: []string{"blockaffinities", "ipamblocks", "ipamhandles", "networksets", "ipamconfigs"}, Verbs: []string{"get", "list", "create", "update", "delete", "watch"}, }, { diff --git a/pkg/render/kubecontrollers/kube-controllers_test.go b/pkg/render/kubecontrollers/kube-controllers_test.go index 20fde73d4a..39f761c4d6 100644 --- a/pkg/render/kubecontrollers/kube-controllers_test.go +++ b/pkg/render/kubecontrollers/kube-controllers_test.go @@ -186,7 +186,7 @@ var _ = Describe("kube-controllers rendering tests", func() { // Verify env expectedEnv := []corev1.EnvVar{ {Name: "DATASTORE_TYPE", Value: "kubernetes"}, - {Name: "ENABLED_CONTROLLERS", Value: "node"}, + {Name: "ENABLED_CONTROLLERS", Value: "node,loadbalancer"}, {Name: "KUBE_CONTROLLERS_CONFIG_NAME", Value: "default"}, {Name: "FIPS_MODE_ENABLED", Value: "false"}, {Name: "DISABLE_KUBE_CONTROLLERS_CONFIG_API", Value: "false"}, @@ -249,14 +249,14 @@ var _ = Describe("kube-controllers rendering tests", func() { Expect(dp.Spec.Template.Spec.Containers[0].Image).To(Equal("test-reg/tigera/kube-controllers:" + components.ComponentTigeraKubeControllers.Version)) envs := dp.Spec.Template.Spec.Containers[0].Env Expect(envs).To(ContainElement(corev1.EnvVar{ - Name: "ENABLED_CONTROLLERS", Value: "node,service,federatedservices,usage", + Name: "ENABLED_CONTROLLERS", Value: "node,service,federatedservices,usage,loadbalancer", })) Expect(len(dp.Spec.Template.Spec.Containers[0].VolumeMounts)).To(Equal(1)) Expect(len(dp.Spec.Template.Spec.Volumes)).To(Equal(1)) clusterRole := rtest.GetResource(resources, kubecontrollers.KubeControllerRole, "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole) - Expect(clusterRole.Rules).To(HaveLen(20)) + Expect(clusterRole.Rules).To(HaveLen(21)) ms := rtest.GetResource(resources, kubecontrollers.KubeControllerMetrics, common.CalicoNamespace, "", "v1", "Service").(*corev1.Service) Expect(ms.Spec.ClusterIP).To(Equal("None"), "metrics service should be headless") @@ -343,7 +343,7 @@ var _ = Describe("kube-controllers rendering tests", func() { Expect(dp.Spec.Template.Spec.Volumes[0].ConfigMap.Name).To(Equal("tigera-ca-bundle")) clusterRole := rtest.GetResource(resources, kubecontrollers.EsKubeControllerRole, "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole) - Expect(clusterRole.Rules).To(HaveLen(18)) + Expect(clusterRole.Rules).To(HaveLen(19)) Expect(clusterRole.Rules).To(ContainElement( rbacv1.PolicyRule{ APIGroups: []string{""}, @@ -390,7 +390,7 @@ var _ = Describe("kube-controllers rendering tests", func() { envs := dp.Spec.Template.Spec.Containers[0].Env Expect(envs).To(ContainElement(corev1.EnvVar{ Name: "ENABLED_CONTROLLERS", - Value: "node,service,federatedservices,usage", + Value: "node,service,federatedservices,usage,loadbalancer", })) Expect(len(dp.Spec.Template.Spec.Containers[0].VolumeMounts)).To(Equal(1)) @@ -546,7 +546,7 @@ var _ = Describe("kube-controllers rendering tests", func() { Expect(dp.Spec.Template.Spec.Containers[0].Image).To(Equal("test-reg/tigera/kube-controllers:" + components.ComponentTigeraKubeControllers.Version)) clusterRole := rtest.GetResource(resources, kubecontrollers.EsKubeControllerRole, "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole) - Expect(clusterRole.Rules).To(HaveLen(18)) + Expect(clusterRole.Rules).To(HaveLen(19)) Expect(clusterRole.Rules).To(ContainElement( rbacv1.PolicyRule{ APIGroups: []string{""}, From b6e81e75048a2e51312c5e2da4dfcd3b69db813b Mon Sep 17 00:00:00 2001 From: Michal Fupso Date: Wed, 11 Sep 2024 14:48:17 -0700 Subject: [PATCH 2/2] fix ut --- pkg/render/kubecontrollers/kube-controllers_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/render/kubecontrollers/kube-controllers_test.go b/pkg/render/kubecontrollers/kube-controllers_test.go index 39f761c4d6..70434e6685 100644 --- a/pkg/render/kubecontrollers/kube-controllers_test.go +++ b/pkg/render/kubecontrollers/kube-controllers_test.go @@ -249,7 +249,7 @@ var _ = Describe("kube-controllers rendering tests", func() { Expect(dp.Spec.Template.Spec.Containers[0].Image).To(Equal("test-reg/tigera/kube-controllers:" + components.ComponentTigeraKubeControllers.Version)) envs := dp.Spec.Template.Spec.Containers[0].Env Expect(envs).To(ContainElement(corev1.EnvVar{ - Name: "ENABLED_CONTROLLERS", Value: "node,service,federatedservices,usage,loadbalancer", + Name: "ENABLED_CONTROLLERS", Value: "node,loadbalancer,service,federatedservices,usage", })) Expect(len(dp.Spec.Template.Spec.Containers[0].VolumeMounts)).To(Equal(1)) @@ -390,7 +390,7 @@ var _ = Describe("kube-controllers rendering tests", func() { envs := dp.Spec.Template.Spec.Containers[0].Env Expect(envs).To(ContainElement(corev1.EnvVar{ Name: "ENABLED_CONTROLLERS", - Value: "node,service,federatedservices,usage,loadbalancer", + Value: "node,loadbalancer,service,federatedservices,usage", })) Expect(len(dp.Spec.Template.Spec.Containers[0].VolumeMounts)).To(Equal(1))