Skip to content

Commit

Permalink
Update operator to enable LoadBalancer kube-controller (#3490)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichalFupso authored Dec 10, 2024
1 parent 37d1641 commit fa0b63b
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 16 deletions.
12 changes: 10 additions & 2 deletions api/v1/installation_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,13 +698,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.
Expand Down Expand Up @@ -761,6 +765,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
}

Expand Down Expand Up @@ -816,6 +822,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.
Expand Down
15 changes: 13 additions & 2 deletions pkg/apis/crd.projectcalico.org/v1/ippool.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/ippool/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ func fillDefaults(ctx context.Context, client client.Client, instance *operator.
}
}
}

if pool.AssignmentMode == "" {
pool.AssignmentMode = crdv1.Automatic
}
}
return nil
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/controller/ippool/pool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
}),
Expand Down Expand Up @@ -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",
},
},
}),
Expand Down Expand Up @@ -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",
},
},
}),
Expand Down
38 changes: 35 additions & 3 deletions pkg/controller/ippool/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ","))
Expand All @@ -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.
Expand All @@ -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",
Expand Down Expand Up @@ -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
}
1 change: 1 addition & 0 deletions pkg/controller/migration/convert/ippools.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
10 changes: 10 additions & 0 deletions pkg/crds/operator/operator.tigera.io_installations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1256,6 +1256,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
Expand Down Expand Up @@ -9608,6 +9613,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
Expand Down
9 changes: 7 additions & 2 deletions pkg/render/kubecontrollers/kube-controllers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -360,6 +360,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"},
Expand All @@ -368,7 +373,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"},
},
{
Expand Down
12 changes: 6 additions & 6 deletions pkg/render/kubecontrollers/kube-controllers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,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: "DISABLE_KUBE_CONTROLLERS_CONFIG_API", Value: "false"},
}
Expand Down Expand Up @@ -247,14 +247,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,loadbalancer,service,federatedservices,usage",
}))

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(19))
Expect(clusterRole.Rules).To(HaveLen(20))

ms := rtest.GetResource(resources, kubecontrollers.KubeControllerMetrics, common.CalicoNamespace, "", "v1", "Service").(*corev1.Service)
Expect(ms.Spec.ClusterIP).To(Equal("None"), "metrics service should be headless")
Expand Down Expand Up @@ -341,7 +341,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{""},
Expand Down Expand Up @@ -388,7 +388,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,loadbalancer,service,federatedservices,usage",
}))

Expect(len(dp.Spec.Template.Spec.Containers[0].VolumeMounts)).To(Equal(1))
Expand Down Expand Up @@ -544,7 +544,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{""},
Expand Down
7 changes: 6 additions & 1 deletion test/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ var _ = Describe("IPPool FV tests", func() {
Expect(ipPools.Items[0].Spec.Disabled).To(Equal(false))
Expect(ipPools.Items[0].Spec.BlockSize).To(Equal(26))
Expect(ipPools.Items[0].Spec.NodeSelector).To(Equal("all()"))
Expect(ipPools.Items[0].Spec.AssignmentMode).To(Equal(crdv1.Automatic))

// Verify that a default IPv6 pool was created.
Expect(ipPools.Items[1].Name).To(Equal("default-ipv6-ippool"))
Expand All @@ -150,6 +151,7 @@ var _ = Describe("IPPool FV tests", func() {
Expect(ipPools.Items[1].Spec.Disabled).To(Equal(false))
Expect(ipPools.Items[1].Spec.BlockSize).To(Equal(122))
Expect(ipPools.Items[1].Spec.NodeSelector).To(Equal("all()"))
Expect(ipPools.Items[1].Spec.AssignmentMode).To(Equal(crdv1.Automatic))

// Expect the default pools to be marked as managed by the operator.
for _, p := range ipPools.Items {
Expand Down Expand Up @@ -190,6 +192,7 @@ var _ = Describe("IPPool FV tests", func() {
Expect(ipPools.Items[0].Spec.BlockSize).To(Equal(26))
Expect(ipPools.Items[0].Spec.NodeSelector).To(Equal("all()"))
Expect(ipPools.Items[0].Labels).To(HaveLen(1))
Expect(ipPools.Items[0].Spec.AssignmentMode).To(Equal(crdv1.Automatic))
})

It("should assume ownership of legacy default IP pools", func() {
Expand All @@ -209,6 +212,7 @@ var _ = Describe("IPPool FV tests", func() {
crdv1.IPPoolAllowedUseWorkload,
crdv1.IPPoolAllowedUseTunnel,
},
AssignmentMode: crdv1.Automatic,
},
}
Expect(c.Create(context.Background(), &ipPool)).To(Succeed())
Expand Down Expand Up @@ -283,6 +287,7 @@ var _ = Describe("IPPool FV tests", func() {
Expect(v3Pools.Items[0].Spec.NodeSelector).To(Equal("all()"))
Expect(v3Pools.Items[0].Spec.IPIPMode).To(Equal(v3.IPIPMode(v3.IPIPModeAlways)))
Expect(v3Pools.Items[0].Spec.VXLANMode).To(Equal(v3.VXLANMode(v3.VXLANModeNever)))
Expect(ipPools.Items[0].Spec.AssignmentMode).To(Equal(crdv1.Automatic))
})

// This test verifies that the IP pool controller doesn't assume ownership of IP pools that may exist in the
Expand Down Expand Up @@ -311,7 +316,7 @@ var _ = Describe("IPPool FV tests", func() {
}
Expect(c.Create(context.Background(), &ipPool)).To(Succeed())

// Create an Installation referencing the IP pool by CIDR, mimicing the upgrade case. We expect
// Create an Installation referencing the IP pool by CIDR, mimicking the upgrade case. We expect
// the operator to default the IP pool, filling in any missing fields. But it won't
// update IP pool in the cluster since it doesn't match exactly.
spec := operator.InstallationSpec{
Expand Down

0 comments on commit fa0b63b

Please sign in to comment.