Skip to content

Commit 290e7c7

Browse files
authored
Allow changing cluster-name on existing deployments (kubernetes#2552)
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.
1 parent 3225d98 commit 290e7c7

File tree

6 files changed

+333
-25
lines changed

6 files changed

+333
-25
lines changed

pkg/openstack/events.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ const (
2222
eventLBSourceRangesIgnored = "LoadBalancerSourceRangesIgnored"
2323
eventLBAZIgnored = "LoadBalancerAvailabilityZonesIgnored"
2424
eventLBFloatingIPSkipped = "LoadBalancerFloatingIPSkipped"
25+
eventLBRename = "LoadBalancerRename"
2526
)

pkg/openstack/loadbalancer.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ import (
4949
// Note: when creating a new Loadbalancer (VM), it can take some time before it is ready for use,
5050
// this timeout is used for waiting until the Loadbalancer provisioning status goes to ACTIVE state.
5151
const (
52-
servicePrefix = "kube_service_"
5352
defaultLoadBalancerSourceRangesIPv4 = "0.0.0.0/0"
5453
defaultLoadBalancerSourceRangesIPv6 = "::/0"
5554
activeStatus = "ACTIVE"
@@ -93,10 +92,14 @@ const (
9392
ServiceAnnotationLoadBalancerID = "loadbalancer.openstack.org/load-balancer-id"
9493

9594
// Octavia resources name formats
95+
servicePrefix = "kube_service_"
9696
lbFormat = "%s%s_%s_%s"
97-
listenerFormat = "listener_%d_%s"
98-
poolFormat = "pool_%d_%s"
99-
monitorFormat = "monitor_%d_%s"
97+
listenerPrefix = "listener_"
98+
listenerFormat = listenerPrefix + "%d_%s"
99+
poolPrefix = "pool_"
100+
poolFormat = poolPrefix + "%d_%s"
101+
monitorPrefix = "monitor_"
102+
monitorFormat = monitorPrefix + "%d_%s"
100103
)
101104

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

1553-
func (lbaas *LbaasV2) updateServiceAnnotations(service *corev1.Service, annotations map[string]string) {
1556+
func (lbaas *LbaasV2) updateServiceAnnotation(service *corev1.Service, key, value string) {
15541557
if service.ObjectMeta.Annotations == nil {
15551558
service.ObjectMeta.Annotations = map[string]string{}
15561559
}
1557-
for key, value := range annotations {
1558-
service.ObjectMeta.Annotations[key] = value
1559-
}
1560+
service.ObjectMeta.Annotations[key] = value
15601561
}
15611562

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

1612+
// Here we test for a clusterName that could have had changed in the deployment.
1613+
if lbHasOldClusterName(loadbalancer, clusterName) {
1614+
msg := "Loadbalancer %s has a name of %s with incorrect cluster-name component. Renaming it to %s."
1615+
klog.Infof(msg, loadbalancer.ID, loadbalancer.Name, lbName)
1616+
lbaas.eventRecorder.Eventf(service, corev1.EventTypeWarning, eventLBRename, msg, loadbalancer.ID, loadbalancer.Name, lbName)
1617+
loadbalancer, err = renameLoadBalancer(lbaas.lb, loadbalancer, lbName, clusterName)
1618+
if err != nil {
1619+
return nil, fmt.Errorf("failed to update load balancer %s with an updated name", svcConf.lbID)
1620+
}
1621+
}
1622+
16111623
// If this LB name matches the default generated name, the Service 'owns' the LB, but it's also possible for this
16121624
// LB to be shared by other Services.
16131625
// If the names don't match, this is a LB this Service wants to attach.
@@ -1656,6 +1668,9 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
16561668
isLBOwner = true
16571669
}
16581670

1671+
// Make sure LB ID will be saved at this point.
1672+
lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerID, loadbalancer.ID)
1673+
16591674
if loadbalancer.ProvisioningStatus != activeStatus {
16601675
return nil, fmt.Errorf("load balancer %s is not ACTIVE, current provisioning status: %s", loadbalancer.ID, loadbalancer.ProvisioningStatus)
16611676
}
@@ -1722,12 +1737,11 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
17221737
return nil, err
17231738
}
17241739
}
1725-
// Add annotation to Service and add LB name to load balancer tags.
1726-
annotationUpdate := map[string]string{
1727-
ServiceAnnotationLoadBalancerID: loadbalancer.ID,
1728-
ServiceAnnotationLoadBalancerAddress: addr,
1729-
}
1730-
lbaas.updateServiceAnnotations(service, annotationUpdate)
1740+
1741+
// save address into the annotation
1742+
lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerAddress, addr)
1743+
1744+
// add LB name to load balancer tags.
17311745
if svcConf.supportLBTags {
17321746
lbTags := loadbalancer.Tags
17331747
if !cpoutil.Contains(lbTags, lbName) {
Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package openstack
18+
19+
import (
20+
"fmt"
21+
"k8s.io/cloud-provider-openstack/pkg/util"
22+
"regexp"
23+
"strings"
24+
25+
"github.com/gophercloud/gophercloud"
26+
27+
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/listeners"
28+
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers"
29+
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/monitors"
30+
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/pools"
31+
openstackutil "k8s.io/cloud-provider-openstack/pkg/util/openstack"
32+
)
33+
34+
// lbHasOldClusterName checks if the OCCM LB prefix is present and if so, validates the cluster-name
35+
// component value. Returns true if the cluster-name component of the loadbalancer's name doesn't match
36+
// clusterName.
37+
func lbHasOldClusterName(loadbalancer *loadbalancers.LoadBalancer, clusterName string) bool {
38+
if !strings.HasPrefix(loadbalancer.Name, servicePrefix) {
39+
// This one was probably not created by OCCM, let's leave it as is.
40+
return false
41+
}
42+
existingClusterName := getClusterName("", loadbalancer.Name)
43+
44+
return existingClusterName != clusterName
45+
}
46+
47+
// decomposeLBName returns clusterName based on object name
48+
func getClusterName(resourcePrefix, objectName string) string {
49+
// This is not 100% bulletproof when string was cut because full name would exceed 255 characters, but honestly
50+
// it's highly unlikely, because it would mean cluster name, namespace and name would together need to exceed 200
51+
// characters. As a precaution the _<name> part is treated as optional in the regexp, assuming the longest trim
52+
// that can happen will reach namespace, but never the clusterName. This fails if there's _ in clusterName, but
53+
// we can't do nothing about it.
54+
lbNameRegex := regexp.MustCompile(fmt.Sprintf("%s%s(.+?)_[^_]+(_[^_]+)?$", resourcePrefix, servicePrefix)) // this is static
55+
56+
matches := lbNameRegex.FindAllStringSubmatch(objectName, -1)
57+
if matches == nil {
58+
return ""
59+
}
60+
return matches[0][1]
61+
}
62+
63+
// replaceClusterName tries to cut servicePrefix, replaces clusterName and puts back the prefix if it was there
64+
func replaceClusterName(oldClusterName, clusterName, objectName string) string {
65+
// Remove the prefix first
66+
var found bool
67+
objectName, found = strings.CutPrefix(objectName, servicePrefix)
68+
objectName = strings.Replace(objectName, oldClusterName, clusterName, 1)
69+
if found {
70+
// This should always happen because we check that in lbHasOldClusterName, but just for safety.
71+
objectName = servicePrefix + objectName
72+
}
73+
// We need to cut the name or tag to 255 characters, just as regular LB creation does.
74+
return util.CutString255(objectName)
75+
}
76+
77+
// renameLoadBalancer renames all the children and then the LB itself to match new lbName.
78+
// The purpose is handling a change of clusterName.
79+
func renameLoadBalancer(client *gophercloud.ServiceClient, loadbalancer *loadbalancers.LoadBalancer, lbName, clusterName string) (*loadbalancers.LoadBalancer, error) {
80+
lbListeners, err := openstackutil.GetListenersByLoadBalancerID(client, loadbalancer.ID)
81+
if err != nil {
82+
return nil, err
83+
}
84+
for _, listener := range lbListeners {
85+
if !strings.HasPrefix(listener.Name, listenerPrefix) {
86+
// It doesn't seem to be ours, let's not touch it.
87+
continue
88+
}
89+
90+
oldClusterName := getClusterName(fmt.Sprintf("%s[0-9]+_", listenerPrefix), listener.Name)
91+
92+
if oldClusterName != clusterName {
93+
// First let's handle pool which we assume is a child of the listener. Only one pool per one listener.
94+
lbPool, err := openstackutil.GetPoolByListener(client, loadbalancer.ID, listener.ID)
95+
if err != nil {
96+
return nil, err
97+
}
98+
oldClusterName = getClusterName(fmt.Sprintf("%s[0-9]+_", poolPrefix), lbPool.Name)
99+
if oldClusterName != clusterName {
100+
if lbPool.MonitorID != "" {
101+
// If monitor exists, let's handle it first, as we treat it as child of the pool.
102+
monitor, err := openstackutil.GetHealthMonitor(client, lbPool.MonitorID)
103+
if err != nil {
104+
return nil, err
105+
}
106+
oldClusterName := getClusterName(fmt.Sprintf("%s[0-9]+_", monitorPrefix), monitor.Name)
107+
if oldClusterName != clusterName {
108+
monitor.Name = replaceClusterName(oldClusterName, clusterName, monitor.Name)
109+
err = openstackutil.UpdateHealthMonitor(client, monitor.ID, monitors.UpdateOpts{Name: &monitor.Name}, loadbalancer.ID)
110+
if err != nil {
111+
return nil, err
112+
}
113+
}
114+
}
115+
116+
// Monitor is handled, let's rename the pool.
117+
lbPool.Name = replaceClusterName(oldClusterName, clusterName, lbPool.Name)
118+
err = openstackutil.UpdatePool(client, loadbalancer.ID, lbPool.ID, pools.UpdateOpts{Name: &lbPool.Name})
119+
if err != nil {
120+
return nil, err
121+
}
122+
}
123+
124+
for i, tag := range listener.Tags {
125+
// There might be tags for shared listeners, that's why we analyze each tag on its own.
126+
oldClusterNameTag := getClusterName("", tag)
127+
if oldClusterNameTag != "" && oldClusterNameTag != clusterName {
128+
listener.Tags[i] = replaceClusterName(oldClusterNameTag, clusterName, tag)
129+
}
130+
}
131+
listener.Name = replaceClusterName(oldClusterName, clusterName, listener.Name)
132+
err = openstackutil.UpdateListener(client, loadbalancer.ID, listener.ID, listeners.UpdateOpts{Name: &listener.Name, Tags: &listener.Tags})
133+
if err != nil {
134+
return nil, err
135+
}
136+
}
137+
}
138+
139+
// At last we rename the LB. This is to make sure we only stop retrying to rename the LB once all of the children
140+
// are handled.
141+
for i, tag := range loadbalancer.Tags {
142+
// There might be tags for shared lbs, that's why we analyze each tag on its own.
143+
oldClusterNameTag := getClusterName("", tag)
144+
if oldClusterNameTag != "" && oldClusterNameTag != clusterName {
145+
loadbalancer.Tags[i] = replaceClusterName(oldClusterNameTag, clusterName, tag)
146+
}
147+
}
148+
return openstackutil.UpdateLoadBalancer(client, loadbalancer.ID, loadbalancers.UpdateOpts{Name: &lbName, Tags: &loadbalancer.Tags})
149+
}
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package openstack
18+
19+
import (
20+
"k8s.io/cloud-provider-openstack/pkg/util"
21+
"strings"
22+
"testing"
23+
24+
"github.com/stretchr/testify/assert"
25+
)
26+
27+
func TestReplaceClusterName(t *testing.T) {
28+
tests := []struct {
29+
name string
30+
oldClusterName string
31+
clusterName string
32+
objectName string
33+
expected string
34+
}{
35+
{
36+
name: "Simple kubernetes replace",
37+
oldClusterName: "kubernetes",
38+
clusterName: "cluster123",
39+
objectName: "kube_service_kubernetes_namespace_name",
40+
expected: "kube_service_cluster123_namespace_name",
41+
},
42+
{
43+
name: "Simple kube replace",
44+
oldClusterName: "kube",
45+
clusterName: "cluster123",
46+
objectName: "kube_service_kube_namespace_name",
47+
expected: "kube_service_cluster123_namespace_name",
48+
},
49+
{
50+
name: "Replace, no prefix",
51+
oldClusterName: "kubernetes",
52+
clusterName: "cluster123",
53+
objectName: "foobar_kubernetes_namespace_name",
54+
expected: "foobar_cluster123_namespace_name",
55+
},
56+
{
57+
name: "Replace, not found",
58+
oldClusterName: "foobar",
59+
clusterName: "cluster123",
60+
objectName: "kube_service_kubernetes_namespace_name",
61+
expected: "kube_service_kubernetes_namespace_name",
62+
},
63+
{
64+
name: "Replace, cut 255",
65+
oldClusterName: "kubernetes",
66+
clusterName: "cluster123",
67+
objectName: "kube_service_kubernetes_namespace_name" + strings.Repeat("foo", 100),
68+
expected: "kube_service_cluster123_namespace_namefoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" +
69+
"foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoo" +
70+
"foofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoofoof",
71+
},
72+
}
73+
74+
for _, test := range tests {
75+
t.Run(test.name, func(t *testing.T) {
76+
result := replaceClusterName(test.oldClusterName, test.clusterName, test.objectName)
77+
assert.Equal(t, test.expected, result)
78+
})
79+
}
80+
}
81+
82+
func TestDecomposeLBName(t *testing.T) {
83+
tests := []struct {
84+
name string
85+
resourcePrefix string
86+
objectName string
87+
expected string
88+
}{
89+
{
90+
name: "Simple kubernetes",
91+
resourcePrefix: "",
92+
objectName: "kube_service_kubernetes_namespace_name",
93+
expected: "kubernetes",
94+
},
95+
{
96+
name: "Kubernetes with prefix",
97+
resourcePrefix: "listener_",
98+
objectName: "listener_kube_service_kubernetes_namespace_name",
99+
expected: "kubernetes",
100+
},
101+
{
102+
name: "Example with _ in clusterName",
103+
resourcePrefix: "listener_",
104+
objectName: "listener_kube_service_kubernetes_123_namespace_name",
105+
expected: "kubernetes_123",
106+
},
107+
{
108+
name: "No match",
109+
resourcePrefix: "listener_",
110+
objectName: "FOOBAR",
111+
expected: "",
112+
},
113+
{
114+
name: "Looong namespace, so string is cut, but no _ in clusterName",
115+
resourcePrefix: "listener_",
116+
objectName: util.CutString255("listener_kube_service_kubernetes_namespace" + strings.Repeat("foo", 100) + "_name"),
117+
expected: "kubernetes",
118+
},
119+
}
120+
121+
for _, test := range tests {
122+
t.Run(test.name, func(t *testing.T) {
123+
result := getClusterName(test.resourcePrefix, test.objectName)
124+
assert.Equal(t, test.expected, result)
125+
})
126+
}
127+
}

pkg/openstack/loadbalancer_test.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,13 +1231,8 @@ func TestLbaasV2_updateServiceAnnotations(t *testing.T) {
12311231
},
12321232
}
12331233

1234-
annotations := map[string]string{
1235-
"key1": "value1",
1236-
"key2": "value2",
1237-
}
1238-
12391234
lbaas := LbaasV2{}
1240-
lbaas.updateServiceAnnotations(service, annotations)
1235+
lbaas.updateServiceAnnotation(service, "key1", "value1")
12411236

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

12471242
expectedAnnotations := []map[string]string{
12481243
{"key1": "value1"},
1249-
{"key2": "value2"},
12501244
}
12511245

12521246
assert.ElementsMatch(t, expectedAnnotations, serviceAnnotations)

0 commit comments

Comments
 (0)