Skip to content

Commit

Permalink
Allow changing cluster-name on existing deployments (#2552)
Browse files Browse the repository at this point in the history
It's a common issue that clusters are deployed with the default
`--cluster-name=kubernetes` and later on it's discovered that next
deployments on the same cloud will have conflicts when trying to manage
LBs of the same namespace and name.

This commit aims at allowing to change the cluster-name on a running
environment and handling all the renames of the LB resources and their
tags.
  • Loading branch information
dulek authored Apr 8, 2024
1 parent 3225d98 commit 290e7c7
Show file tree
Hide file tree
Showing 6 changed files with 333 additions and 25 deletions.
1 change: 1 addition & 0 deletions pkg/openstack/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ const (
eventLBSourceRangesIgnored = "LoadBalancerSourceRangesIgnored"
eventLBAZIgnored = "LoadBalancerAvailabilityZonesIgnored"
eventLBFloatingIPSkipped = "LoadBalancerFloatingIPSkipped"
eventLBRename = "LoadBalancerRename"
)
42 changes: 28 additions & 14 deletions pkg/openstack/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import (
// Note: when creating a new Loadbalancer (VM), it can take some time before it is ready for use,
// this timeout is used for waiting until the Loadbalancer provisioning status goes to ACTIVE state.
const (
servicePrefix = "kube_service_"
defaultLoadBalancerSourceRangesIPv4 = "0.0.0.0/0"
defaultLoadBalancerSourceRangesIPv6 = "::/0"
activeStatus = "ACTIVE"
Expand Down Expand Up @@ -93,10 +92,14 @@ const (
ServiceAnnotationLoadBalancerID = "loadbalancer.openstack.org/load-balancer-id"

// Octavia resources name formats
servicePrefix = "kube_service_"
lbFormat = "%s%s_%s_%s"
listenerFormat = "listener_%d_%s"
poolFormat = "pool_%d_%s"
monitorFormat = "monitor_%d_%s"
listenerPrefix = "listener_"
listenerFormat = listenerPrefix + "%d_%s"
poolPrefix = "pool_"
poolFormat = poolPrefix + "%d_%s"
monitorPrefix = "monitor_"
monitorFormat = monitorPrefix + "%d_%s"
)

// LbaasV2 is a LoadBalancer implementation based on Octavia
Expand Down Expand Up @@ -1550,13 +1553,11 @@ func (lbaas *LbaasV2) checkListenerPorts(service *corev1.Service, curListenerMap
return nil
}

func (lbaas *LbaasV2) updateServiceAnnotations(service *corev1.Service, annotations map[string]string) {
func (lbaas *LbaasV2) updateServiceAnnotation(service *corev1.Service, key, value string) {
if service.ObjectMeta.Annotations == nil {
service.ObjectMeta.Annotations = map[string]string{}
}
for key, value := range annotations {
service.ObjectMeta.Annotations[key] = value
}
service.ObjectMeta.Annotations[key] = value
}

// createLoadBalancerStatus creates the loadbalancer status from the different possible sources
Expand Down Expand Up @@ -1608,6 +1609,17 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
return nil, fmt.Errorf("failed to get load balancer %s: %v", svcConf.lbID, err)
}

// Here we test for a clusterName that could have had changed in the deployment.
if lbHasOldClusterName(loadbalancer, clusterName) {
msg := "Loadbalancer %s has a name of %s with incorrect cluster-name component. Renaming it to %s."
klog.Infof(msg, loadbalancer.ID, loadbalancer.Name, lbName)
lbaas.eventRecorder.Eventf(service, corev1.EventTypeWarning, eventLBRename, msg, loadbalancer.ID, loadbalancer.Name, lbName)
loadbalancer, err = renameLoadBalancer(lbaas.lb, loadbalancer, lbName, clusterName)
if err != nil {
return nil, fmt.Errorf("failed to update load balancer %s with an updated name", svcConf.lbID)
}
}

// If this LB name matches the default generated name, the Service 'owns' the LB, but it's also possible for this
// LB to be shared by other Services.
// If the names don't match, this is a LB this Service wants to attach.
Expand Down Expand Up @@ -1656,6 +1668,9 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
isLBOwner = true
}

// Make sure LB ID will be saved at this point.
lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerID, loadbalancer.ID)

if loadbalancer.ProvisioningStatus != activeStatus {
return nil, fmt.Errorf("load balancer %s is not ACTIVE, current provisioning status: %s", loadbalancer.ID, loadbalancer.ProvisioningStatus)
}
Expand Down Expand Up @@ -1722,12 +1737,11 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
return nil, err
}
}
// Add annotation to Service and add LB name to load balancer tags.
annotationUpdate := map[string]string{
ServiceAnnotationLoadBalancerID: loadbalancer.ID,
ServiceAnnotationLoadBalancerAddress: addr,
}
lbaas.updateServiceAnnotations(service, annotationUpdate)

// save address into the annotation
lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerAddress, addr)

// add LB name to load balancer tags.
if svcConf.supportLBTags {
lbTags := loadbalancer.Tags
if !cpoutil.Contains(lbTags, lbName) {
Expand Down
149 changes: 149 additions & 0 deletions pkg/openstack/loadbalancer_rename.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package openstack

import (
"fmt"
"k8s.io/cloud-provider-openstack/pkg/util"
"regexp"
"strings"

"github.com/gophercloud/gophercloud"

"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/listeners"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/monitors"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools"
openstackutil "k8s.io/cloud-provider-openstack/pkg/util/openstack"
)

// lbHasOldClusterName checks if the OCCM LB prefix is present and if so, validates the cluster-name
// component value. Returns true if the cluster-name component of the loadbalancer's name doesn't match
// clusterName.
func lbHasOldClusterName(loadbalancer *loadbalancers.LoadBalancer, clusterName string) bool {
if !strings.HasPrefix(loadbalancer.Name, servicePrefix) {
// This one was probably not created by OCCM, let's leave it as is.
return false
}
existingClusterName := getClusterName("", loadbalancer.Name)

return existingClusterName != clusterName
}

// decomposeLBName returns clusterName based on object name
func getClusterName(resourcePrefix, objectName string) string {
// This is not 100% bulletproof when string was cut because full name would exceed 255 characters, but honestly
// it's highly unlikely, because it would mean cluster name, namespace and name would together need to exceed 200
// characters. As a precaution the _<name> part is treated as optional in the regexp, assuming the longest trim
// that can happen will reach namespace, but never the clusterName. This fails if there's _ in clusterName, but
// we can't do nothing about it.
lbNameRegex := regexp.MustCompile(fmt.Sprintf("%s%s(.+?)_[^_]+(_[^_]+)?$", resourcePrefix, servicePrefix)) // this is static

matches := lbNameRegex.FindAllStringSubmatch(objectName, -1)
if matches == nil {
return ""
}
return matches[0][1]
}

// replaceClusterName tries to cut servicePrefix, replaces clusterName and puts back the prefix if it was there
func replaceClusterName(oldClusterName, clusterName, objectName string) string {
// Remove the prefix first
var found bool
objectName, found = strings.CutPrefix(objectName, servicePrefix)
objectName = strings.Replace(objectName, oldClusterName, clusterName, 1)
if found {
// This should always happen because we check that in lbHasOldClusterName, but just for safety.
objectName = servicePrefix + objectName
}
// We need to cut the name or tag to 255 characters, just as regular LB creation does.
return util.CutString255(objectName)
}

// renameLoadBalancer renames all the children and then the LB itself to match new lbName.
// The purpose is handling a change of clusterName.
func renameLoadBalancer(client *gophercloud.ServiceClient, loadbalancer *loadbalancers.LoadBalancer, lbName, clusterName string) (*loadbalancers.LoadBalancer, error) {
lbListeners, err := openstackutil.GetListenersByLoadBalancerID(client, loadbalancer.ID)
if err != nil {
return nil, err
}
for _, listener := range lbListeners {
if !strings.HasPrefix(listener.Name, listenerPrefix) {
// It doesn't seem to be ours, let's not touch it.
continue
}

oldClusterName := getClusterName(fmt.Sprintf("%s[0-9]+_", listenerPrefix), listener.Name)

if oldClusterName != clusterName {
// First let's handle pool which we assume is a child of the listener. Only one pool per one listener.
lbPool, err := openstackutil.GetPoolByListener(client, loadbalancer.ID, listener.ID)
if err != nil {
return nil, err
}
oldClusterName = getClusterName(fmt.Sprintf("%s[0-9]+_", poolPrefix), lbPool.Name)
if oldClusterName != clusterName {
if lbPool.MonitorID != "" {
// If monitor exists, let's handle it first, as we treat it as child of the pool.
monitor, err := openstackutil.GetHealthMonitor(client, lbPool.MonitorID)
if err != nil {
return nil, err
}
oldClusterName := getClusterName(fmt.Sprintf("%s[0-9]+_", monitorPrefix), monitor.Name)
if oldClusterName != clusterName {
monitor.Name = replaceClusterName(oldClusterName, clusterName, monitor.Name)
err = openstackutil.UpdateHealthMonitor(client, monitor.ID, monitors.UpdateOpts{Name: &monitor.Name}, loadbalancer.ID)
if err != nil {
return nil, err
}
}
}

// Monitor is handled, let's rename the pool.
lbPool.Name = replaceClusterName(oldClusterName, clusterName, lbPool.Name)
err = openstackutil.UpdatePool(client, loadbalancer.ID, lbPool.ID, pools.UpdateOpts{Name: &lbPool.Name})
if err != nil {
return nil, err
}
}

for i, tag := range listener.Tags {
// There might be tags for shared listeners, that's why we analyze each tag on its own.
oldClusterNameTag := getClusterName("", tag)
if oldClusterNameTag != "" && oldClusterNameTag != clusterName {
listener.Tags[i] = replaceClusterName(oldClusterNameTag, clusterName, tag)
}
}
listener.Name = replaceClusterName(oldClusterName, clusterName, listener.Name)
err = openstackutil.UpdateListener(client, loadbalancer.ID, listener.ID, listeners.UpdateOpts{Name: &listener.Name, Tags: &listener.Tags})
if err != nil {
return nil, err
}
}
}

// At last we rename the LB. This is to make sure we only stop retrying to rename the LB once all of the children
// are handled.
for i, tag := range loadbalancer.Tags {
// There might be tags for shared lbs, that's why we analyze each tag on its own.
oldClusterNameTag := getClusterName("", tag)
if oldClusterNameTag != "" && oldClusterNameTag != clusterName {
loadbalancer.Tags[i] = replaceClusterName(oldClusterNameTag, clusterName, tag)
}
}
return openstackutil.UpdateLoadBalancer(client, loadbalancer.ID, loadbalancers.UpdateOpts{Name: &lbName, Tags: &loadbalancer.Tags})
}
127 changes: 127 additions & 0 deletions pkg/openstack/loadbalancer_rename_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
Copyright 2024 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package openstack

import (
"k8s.io/cloud-provider-openstack/pkg/util"
"strings"
"testing"

"github.com/stretchr/testify/assert"
)

func TestReplaceClusterName(t *testing.T) {
tests := []struct {
name string
oldClusterName string
clusterName string
objectName string
expected string
}{
{
name: "Simple kubernetes replace",
oldClusterName: "kubernetes",
clusterName: "cluster123",
objectName: "kube_service_kubernetes_namespace_name",
expected: "kube_service_cluster123_namespace_name",
},
{
name: "Simple kube replace",
oldClusterName: "kube",
clusterName: "cluster123",
objectName: "kube_service_kube_namespace_name",
expected: "kube_service_cluster123_namespace_name",
},
{
name: "Replace, no prefix",
oldClusterName: "kubernetes",
clusterName: "cluster123",
objectName: "foobar_kubernetes_namespace_name",
expected: "foobar_cluster123_namespace_name",
},
{
name: "Replace, not found",
oldClusterName: "foobar",
clusterName: "cluster123",
objectName: "kube_service_kubernetes_namespace_name",
expected: "kube_service_kubernetes_namespace_name",
},
{
name: "Replace, cut 255",
oldClusterName: "kubernetes",
clusterName: "cluster123",
objectName: "kube_service_kubernetes_namespace_name" + strings.Repeat("foo", 100),
expected: "kube_service_cluster123_namespace_namefoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" +
"foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" +
"foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoof",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := replaceClusterName(test.oldClusterName, test.clusterName, test.objectName)
assert.Equal(t, test.expected, result)
})
}
}

func TestDecomposeLBName(t *testing.T) {
tests := []struct {
name string
resourcePrefix string
objectName string
expected string
}{
{
name: "Simple kubernetes",
resourcePrefix: "",
objectName: "kube_service_kubernetes_namespace_name",
expected: "kubernetes",
},
{
name: "Kubernetes with prefix",
resourcePrefix: "listener_",
objectName: "listener_kube_service_kubernetes_namespace_name",
expected: "kubernetes",
},
{
name: "Example with _ in clusterName",
resourcePrefix: "listener_",
objectName: "listener_kube_service_kubernetes_123_namespace_name",
expected: "kubernetes_123",
},
{
name: "No match",
resourcePrefix: "listener_",
objectName: "FOOBAR",
expected: "",
},
{
name: "Looong namespace, so string is cut, but no _ in clusterName",
resourcePrefix: "listener_",
objectName: util.CutString255("listener_kube_service_kubernetes_namespace" + strings.Repeat("foo", 100) + "_name"),
expected: "kubernetes",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := getClusterName(test.resourcePrefix, test.objectName)
assert.Equal(t, test.expected, result)
})
}
}
8 changes: 1 addition & 7 deletions pkg/openstack/loadbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1231,13 +1231,8 @@ func TestLbaasV2_updateServiceAnnotations(t *testing.T) {
},
}

annotations := map[string]string{
"key1": "value1",
"key2": "value2",
}

lbaas := LbaasV2{}
lbaas.updateServiceAnnotations(service, annotations)
lbaas.updateServiceAnnotation(service, "key1", "value1")

serviceAnnotations := make([]map[string]string, 0)
for key, value := range service.ObjectMeta.Annotations {
Expand All @@ -1246,7 +1241,6 @@ func TestLbaasV2_updateServiceAnnotations(t *testing.T) {

expectedAnnotations := []map[string]string{
{"key1": "value1"},
{"key2": "value2"},
}

assert.ElementsMatch(t, expectedAnnotations, serviceAnnotations)
Expand Down
Loading

0 comments on commit 290e7c7

Please sign in to comment.