From c5138aa210e4b7eb28b13cb268a252e7eb9830c4 Mon Sep 17 00:00:00 2001 From: enginrect Date: Thu, 30 Apr 2026 09:51:43 +0900 Subject: [PATCH 1/4] [occm] Tag load balancers with cluster identity to prevent name collisions OCCM constructs Octavia load balancer names as kube_service___. When two Kubernetes clusters share the same OpenStack project and use the same --cluster-name (default "kubernetes"), services with identical namespace/name produce identical load balancer names. Octavia does not enforce uniqueness on load balancer names, so OCCM's first-time name-based lookup can adopt and overwrite a load balancer that actually belongs to a different cluster (see issues #2241, #2571, #2624). This commit adds a stable Kubernetes cluster identifier - the UID of the kube-system namespace - as a load balancer tag of the form kube_cluster_id_. getLoadbalancerByName now ignores load balancers that carry a cluster-id tag for a different cluster and falls back to the legacy behaviour for load balancers without any cluster-id tag, so existing deployments keep working unchanged. Pre-existing load balancers gain the new tag during the next reconciliation. The cluster UID is read once at controller-manager start-up via the kube-system namespace; failure to read it (RBAC denial, missing namespace) is non-fatal and disables the safeguard, falling back to legacy name-based lookup. The cloud-controller-manager ClusterRole and the helm chart gain "get" on namespaces. Made-with: Cursor --- .../templates/clusterrole.yaml | 6 + ...cations-using-loadbalancer-type-service.md | 50 ++++++++ .../cloud-controller-manager-roles.yaml | 6 + pkg/openstack/loadbalancer.go | 112 +++++++++++++++- pkg/openstack/loadbalancer_test.go | 120 ++++++++++++++++++ pkg/openstack/openstack.go | 35 ++++- pkg/openstack/openstack_test.go | 48 +++++++ 7 files changed, 369 insertions(+), 8 deletions(-) diff --git a/charts/openstack-cloud-controller-manager/templates/clusterrole.yaml b/charts/openstack-cloud-controller-manager/templates/clusterrole.yaml index cf03f8a11a..307acaf614 100644 --- a/charts/openstack-cloud-controller-manager/templates/clusterrole.yaml +++ b/charts/openstack-cloud-controller-manager/templates/clusterrole.yaml @@ -96,3 +96,9 @@ rules: - list - get - watch +- apiGroups: + - "" + resources: + - namespaces + verbs: + - get diff --git a/docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md b/docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md index a74a3b44b3..f6e0f0b4d5 100644 --- a/docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md +++ b/docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md @@ -11,6 +11,7 @@ - [Restrict Access For LoadBalancer Service](#restrict-access-for-loadbalancer-service) - [Use PROXY protocol to preserve client IP](#use-proxy-protocol-to-preserve-client-ip) - [Sharing load balancer with multiple Services](#sharing-load-balancer-with-multiple-services) + - [Running multiple Kubernetes clusters in one OpenStack project](#running-multiple-kubernetes-clusters-in-one-openstack-project) - [IPv4 / IPv6 dual-stack services](#ipv4--ipv6-dual-stack-services) @@ -627,6 +628,55 @@ $ openstack loadbalancer listener list --loadbalancer 2b224530-9414-4302-8163-5a The load balancer will be deleted after `service-2` is deleted. +### Running multiple Kubernetes clusters in one OpenStack project + +OCCM names each load balancer it creates as `kube_service___`, +where `` comes from the kube-controller-manager `--cluster-name` flag and +defaults to `kubernetes`. When two Kubernetes clusters share the same OpenStack project +and use the same `--cluster-name`, namespace and service name, the resulting load balancer +names collide. OpenStack does not require load balancer names to be unique, so previously +the second cluster could "adopt" the load balancer that the first cluster owned and start +overwriting its configuration. The recommended way to avoid this remains to set a unique +`--cluster-name` on every Kubernetes cluster (see issues +[#2241](https://github.com/kubernetes/cloud-provider-openstack/issues/2241), +[#2571](https://github.com/kubernetes/cloud-provider-openstack/issues/2571), +[#2624](https://github.com/kubernetes/cloud-provider-openstack/issues/2624)). + +To make OCCM resilient even when that recommendation isn't followed, OCCM also tags +every load balancer it creates with the UID of the cluster's `kube-system` namespace, +which is a stable and unique identifier per Kubernetes cluster. The tag has the form +`kube_cluster_id_`. When OCCM looks up a load balancer by name on the first +reconcile of a Service, it ignores load balancers that carry a `kube_cluster_id_*` tag +for a *different* cluster and creates a new one instead. Load balancers that don't +carry any `kube_cluster_id_*` tag (legacy load balancers, or load balancers created by +external tooling) keep their previous behaviour for backward compatibility, and gain +the cluster-id tag on the next reconcile. + +OCCM reads the `kube-system` namespace UID once at start-up; this requires the +`get` verb on the `namespaces` resource (already part of the standard cloud-controller +RBAC). If the lookup fails (for example because of a custom RBAC restriction) OCCM logs +a warning and continues without the safeguard, falling back to the legacy name-based +behaviour. + +After successful reconcile, the load balancer tags will look similar to: + +```shell +$ openstack loadbalancer show -c name -c tags ++-------+--------------------------------------------------------------------------+ +| Field | Value | ++-------+--------------------------------------------------------------------------+ +| name | kube_service_kubernetes_default_service-1 | +| tags | kube_service_kubernetes_default_service-1, | +| | kube_cluster_id_11111111-2222-3333-4444-555555555555 | ++-------+--------------------------------------------------------------------------+ +``` + +> NOTE: This safeguard only protects new (or freshly reconciled) load balancers. It +> does not retroactively resolve an already-stolen load balancer between two running +> clusters. If you suspect cross-cluster collisions, set unique `--cluster-name` on +> each cluster and let OCCM rename the existing resources (see +> [PR #2552](https://github.com/kubernetes/cloud-provider-openstack/pull/2552)). + ### IPv4 / IPv6 dual-stack services Since Kubernetes 1.20, Kubernetes clusters can run in dual-stack mode, which allows simultaneous usage of both IPv4 and IPv6 addresses in the cluster. diff --git a/manifests/controller-manager/cloud-controller-manager-roles.yaml b/manifests/controller-manager/cloud-controller-manager-roles.yaml index 93a47b74c2..e5c52d698b 100644 --- a/manifests/controller-manager/cloud-controller-manager-roles.yaml +++ b/manifests/controller-manager/cloud-controller-manager-roles.yaml @@ -93,6 +93,12 @@ items: - list - get - watch + - apiGroups: + - "" + resources: + - namespaces + verbs: + - get - apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/pkg/openstack/loadbalancer.go b/pkg/openstack/loadbalancer.go index 0a007e840a..ab8ed0859d 100644 --- a/pkg/openstack/loadbalancer.go +++ b/pkg/openstack/loadbalancer.go @@ -105,8 +105,31 @@ const ( poolFormat = poolPrefix + "%d_%s" monitorPrefix = "monitor_" monitorFormat = monitorPrefix + "%d_%s" + + // clusterIDTagPrefix is the prefix used for the load balancer tag that + // carries a stable Kubernetes cluster identifier (the kube-system + // namespace UID). Together with the existing servicePrefix tag it allows + // OCCM to disambiguate load balancers when multiple Kubernetes clusters + // share the same OpenStack project and a service name happens to collide. + clusterIDTagPrefix = "kube_cluster_id_" + + // eventLBStolen is emitted when getLoadbalancerByName finds a load + // balancer with a matching name that belongs to a different Kubernetes + // cluster (different cluster-id tag). The lookup is treated as NotFound + // so OCCM creates a new load balancer instead of stealing an existing one. + eventLBStolen = "LoadBalancerNameCollision" ) +// clusterIDTag formats the Octavia load balancer tag carrying the cluster +// identifier. It returns the empty string when uid is empty so callers can +// safely append the result without a separate nil-check. +func clusterIDTag(uid string) string { + if uid == "" { + return "" + } + return clusterIDTagPrefix + uid +} + // LbaasV2 is a LoadBalancer implementation based on Octavia type LbaasV2 struct { LoadBalancer @@ -153,8 +176,17 @@ type listenerKey struct { Port int } -// getLoadbalancerByName get the load balancer which is in valid status by the given name/legacy name. -func getLoadbalancerByName(ctx context.Context, client *gophercloud.ServiceClient, name string, legacyName string) (*loadbalancers.LoadBalancer, error) { +// getLoadbalancerByName gets the load balancer which is in valid status by the given name/legacy name. +// +// When clusterUID is non-empty, the returned load balancer must either carry +// the matching clusterIDTagPrefix tag for that UID, or carry no clusterIDTagPrefix +// tag at all (legacy load balancer that pre-dates the tag). Load balancers +// whose name matches but that carry a different cluster-id tag belong to +// another Kubernetes cluster sharing the OpenStack project; they are ignored +// and the lookup returns ErrNotFound, which causes OCCM to create a new load +// balancer instead of accidentally adopting (and overwriting) one that is +// owned by a different cluster. +func getLoadbalancerByName(ctx context.Context, client *gophercloud.ServiceClient, name string, legacyName string, clusterUID string) (*loadbalancers.LoadBalancer, error) { var validLBs []loadbalancers.LoadBalancer opts := loadbalancers.ListOpts{ @@ -187,16 +219,71 @@ func getLoadbalancerByName(ctx context.Context, client *gophercloud.ServiceClien } } + validLBs, foreignFound := filterLoadBalancersByClusterID(validLBs, clusterUID) + if len(validLBs) > 1 { return nil, cpoerrors.ErrMultipleResults } if len(validLBs) == 0 { + if foreignFound { + klog.Warningf("Found a load balancer named %q in OpenStack but it belongs to a different Kubernetes cluster "+ + "(no %s%s tag); ignoring it. A new load balancer will be created.", name, clusterIDTagPrefix, clusterUID) + } return nil, cpoerrors.ErrNotFound } return &validLBs[0], nil } +// filterLoadBalancersByClusterID returns the subset of lbs that may belong to +// the cluster identified by clusterUID. The selection rules are: +// +// - If clusterUID is empty, the filter is a no-op (the caller has no cluster +// identity to match on). +// - Load balancers carrying the matching clusterIDTagPrefix+clusterUID tag +// are kept (strongly owned by this cluster). +// - If none of the load balancers carries any clusterIDTagPrefix tag, all of +// them are kept. This preserves the legacy behaviour for load balancers +// created before the tag was introduced or by tools that don't set it. +// - Otherwise (every candidate carries a foreign clusterIDTagPrefix tag) all +// load balancers are dropped. The second return value is true in that case +// to let the caller emit a more specific log line / event. +func filterLoadBalancersByClusterID(lbs []loadbalancers.LoadBalancer, clusterUID string) ([]loadbalancers.LoadBalancer, bool) { + if clusterUID == "" || len(lbs) == 0 { + return lbs, false + } + wantTag := clusterIDTag(clusterUID) + var owned []loadbalancers.LoadBalancer + taggedAny := false + for _, lb := range lbs { + hasTag := false + match := false + for _, tag := range lb.Tags { + if strings.HasPrefix(tag, clusterIDTagPrefix) { + hasTag = true + if tag == wantTag { + match = true + } + } + } + if match { + owned = append(owned, lb) + } + if hasTag { + taggedAny = true + } + } + if len(owned) > 0 { + return owned, false + } + if !taggedAny { + // Legacy load balancers without any cluster-id tag, behave as before. + return lbs, false + } + // All candidates are tagged for some other cluster. + return nil, true +} + func popListener(existingListeners []listeners.Listener, id string) []listeners.Listener { newListeners := []listeners.Listener{} for _, existingListener := range existingListeners { @@ -236,6 +323,9 @@ func (lbaas *LbaasV2) createOctaviaLoadBalancer(ctx context.Context, name, clust if svcConf.supportLBTags { createOpts.Tags = []string{svcConf.lbName} + if tag := clusterIDTag(lbaas.clusterUID); tag != "" { + createOpts.Tags = append(createOpts.Tags, tag) + } } if svcConf.flavorID != "" { @@ -339,7 +429,7 @@ func (lbaas *LbaasV2) GetLoadBalancer(ctx context.Context, clusterName string, s if lbID != "" { loadbalancer, err = openstackutil.GetLoadbalancerByID(ctx, lbaas.lb, lbID) } else { - loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, name, legacyName) + loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, name, legacyName, lbaas.clusterUID) } if err != nil && cpoerrors.IsNotFound(err) { return nil, false, nil @@ -1737,7 +1827,7 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName } } else { legacyName := lbaas.getLoadBalancerLegacyName(service) - loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, lbName, legacyName) + loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, lbName, legacyName, lbaas.clusterUID) if err != nil { if err != cpoerrors.ErrNotFound { return nil, fmt.Errorf("error getting loadbalancer for Service %s: %v", serviceName, err) @@ -1826,11 +1916,19 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName // save address into the annotation lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerAddress, addr) - // add LB name to load balancer tags. + // add LB name and cluster-identity tags to the load balancer. if svcConf.supportLBTags { lbTags := loadbalancer.Tags + changed := false if !slices.Contains(lbTags, lbName) { lbTags = append(lbTags, lbName) + changed = true + } + if tag := clusterIDTag(lbaas.clusterUID); tag != "" && !slices.Contains(lbTags, tag) { + lbTags = append(lbTags, tag) + changed = true + } + if changed { klog.InfoS("Updating load balancer tags", "lbID", loadbalancer.ID, "tags", lbTags) if err := openstackutil.UpdateLoadBalancerTags(ctx, lbaas.lb, loadbalancer.ID, lbTags); err != nil { return nil, err @@ -1912,7 +2010,7 @@ func (lbaas *LbaasV2) updateOctaviaLoadBalancer(ctx context.Context, clusterName // This is a Service created before shared LB is supported. name := lbaas.GetLoadBalancerName(ctx, clusterName, service) legacyName := lbaas.getLoadBalancerLegacyName(service) - loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, name, legacyName) + loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, name, legacyName, lbaas.clusterUID) if err != nil { return err } @@ -2100,7 +2198,7 @@ func (lbaas *LbaasV2) ensureLoadBalancerDeleted(ctx context.Context, clusterName loadbalancer, err = openstackutil.GetLoadbalancerByID(ctx, lbaas.lb, svcConf.lbID) } else { // This may happen when this Service creation was failed previously. - loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, lbName, legacyName) + loadbalancer, err = getLoadbalancerByName(ctx, lbaas.lb, lbName, legacyName, lbaas.clusterUID) } if err != nil && !cpoerrors.IsNotFound(err) { return err diff --git a/pkg/openstack/loadbalancer_test.go b/pkg/openstack/loadbalancer_test.go index 27304a45ad..f4f46d4f55 100644 --- a/pkg/openstack/loadbalancer_test.go +++ b/pkg/openstack/loadbalancer_test.go @@ -10,6 +10,7 @@ import ( "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/listeners" + "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/loadbalancers" v2monitors "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/monitors" "github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/pools" "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/rules" @@ -2675,3 +2676,122 @@ func Test_getProxyProtocolFromServiceAnnotation(t *testing.T) { }) } } + +func TestClusterIDTag(t *testing.T) { + assert.Equal(t, "", clusterIDTag("")) + assert.Equal(t, "kube_cluster_id_abc-123", clusterIDTag("abc-123")) + assert.True(t, len(clusterIDTagPrefix) > 0) +} + +func TestFilterLoadBalancersByClusterID(t *testing.T) { + const ( + thisUID = "11111111-2222-3333-4444-555555555555" + otherUID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + thisTag = clusterIDTagPrefix + thisUID + otherTag = clusterIDTagPrefix + otherUID + serviceTag = "kube_service_kubernetes_default_test" + ) + + mkLB := func(id string, tags ...string) loadbalancers.LoadBalancer { + return loadbalancers.LoadBalancer{ID: id, Tags: tags} + } + + tests := []struct { + name string + clusterUID string + input []loadbalancers.LoadBalancer + wantIDs []string + wantForeignFnd bool + }{ + { + name: "empty clusterUID is a no-op", + clusterUID: "", + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag), + mkLB("b", serviceTag, otherTag), + }, + wantIDs: []string{"a", "b"}, + wantForeignFnd: false, + }, + { + name: "empty input list", + clusterUID: thisUID, + input: nil, + wantIDs: nil, + wantForeignFnd: false, + }, + { + name: "single matching tag is kept", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag, thisTag), + }, + wantIDs: []string{"a"}, + wantForeignFnd: false, + }, + { + name: "all untagged falls back to legacy behaviour", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag), + }, + wantIDs: []string{"a"}, + wantForeignFnd: false, + }, + { + name: "foreign cluster tag is filtered out", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag, otherTag), + }, + wantIDs: nil, + wantForeignFnd: true, + }, + { + name: "mixed: matching kept, foreign dropped", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag, thisTag), + mkLB("b", serviceTag, otherTag), + }, + wantIDs: []string{"a"}, + wantForeignFnd: false, + }, + { + name: "mixed: untagged ignored when matching exists", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag, thisTag), + mkLB("b", serviceTag), + }, + wantIDs: []string{"a"}, + wantForeignFnd: false, + }, + { + name: "all foreign returns empty with foreignFound true", + clusterUID: thisUID, + input: []loadbalancers.LoadBalancer{ + mkLB("a", serviceTag, otherTag), + mkLB("b", serviceTag, otherTag), + }, + wantIDs: nil, + wantForeignFnd: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, foreign := filterLoadBalancersByClusterID(tt.input, tt.clusterUID) + gotIDs := make([]string, 0, len(got)) + for _, lb := range got { + gotIDs = append(gotIDs, lb.ID) + } + if len(tt.wantIDs) == 0 { + assert.Empty(t, gotIDs) + } else { + assert.Equal(t, tt.wantIDs, gotIDs) + } + assert.Equal(t, tt.wantForeignFnd, foreign) + }) + } +} diff --git a/pkg/openstack/openstack.go b/pkg/openstack/openstack.go index ac07d50345..726ec6b627 100644 --- a/pkg/openstack/openstack.go +++ b/pkg/openstack/openstack.go @@ -30,6 +30,7 @@ import ( neutronports "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" "github.com/spf13/pflag" gcfg "gopkg.in/gcfg.v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" cloudprovider "k8s.io/cloud-provider" "k8s.io/klog/v2" @@ -88,6 +89,12 @@ type LoadBalancer struct { opts LoadBalancerOpts kclient kubernetes.Interface eventRecorder record.EventRecorder + // clusterUID is a stable, cluster-scoped identifier (the kube-system namespace UID). + // It is attached as a tag on Octavia load balancers so that OCCM can disambiguate + // load balancers with the same name when multiple Kubernetes clusters share the + // same OpenStack project. An empty value disables the disambiguation and falls + // back to the legacy name-based behaviour. + clusterUID string } // LoadBalancerOpts have the options to talk to Neutron LBaaSV2 or Octavia @@ -160,6 +167,11 @@ type OpenStack struct { kclient kubernetes.Interface nodeInformer coreinformers.NodeInformer nodeInformerHasSynced func() bool + // clusterUID is the kube-system namespace UID, used as a stable cluster + // identifier on OpenStack load balancer tags. May be empty if the lookup + // failed or RBAC does not allow it; in that case OCCM falls back to the + // legacy name-based load balancer identification. + clusterUID string eventBroadcaster record.EventBroadcaster eventRecorder record.EventRecorder @@ -199,6 +211,27 @@ func (os *OpenStack) Initialize(clientBuilder cloudprovider.ControllerClientBuil os.eventBroadcaster = record.NewBroadcaster() os.eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: os.kclient.CoreV1().Events("")}) os.eventRecorder = os.eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-provider-openstack"}) + os.clusterUID = fetchClusterUID(clientset) +} + +// fetchClusterUID returns the UID of the kube-system namespace, which is a +// stable, unique identifier for the Kubernetes cluster. It is used as a tag on +// Octavia load balancers so OCCM can tell its own load balancers apart from +// load balancers created by a different cluster sharing the same OpenStack +// project even when the names collide. Failure is non-fatal: callers must +// tolerate an empty string and fall back to the legacy behaviour. +func fetchClusterUID(clientset kubernetes.Interface) string { + ctx, cancel := context.WithTimeout(context.Background(), defaultTimeOut) + defer cancel() + + ns, err := clientset.CoreV1().Namespaces().Get(ctx, "kube-system", metav1.GetOptions{}) + if err != nil { + klog.Warningf("Failed to read kube-system namespace UID, load balancer cluster-identity tagging will be disabled: %v", err) + return "" + } + uid := string(ns.UID) + klog.V(2).Infof("Using kube-system namespace UID %q as cluster identity for load balancer tagging", uid) + return uid } // ReadConfig reads values from the cloud.conf @@ -372,7 +405,7 @@ func (os *OpenStack) LoadBalancer() (cloudprovider.LoadBalancer, bool) { klog.V(1).Info("Claiming to support LoadBalancer") - return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder}}, true + return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, os.clusterUID}}, true } // Zones indicates that we support zones diff --git a/pkg/openstack/openstack_test.go b/pkg/openstack/openstack_test.go index 3291a8d0c8..d506b85f50 100644 --- a/pkg/openstack/openstack_test.go +++ b/pkg/openstack/openstack_test.go @@ -18,6 +18,7 @@ package openstack import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -30,7 +31,13 @@ import ( neutronports "github.com/gophercloud/gophercloud/v2/openstack/networking/v2/ports" "github.com/spf13/pflag" v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" "k8s.io/cloud-provider-openstack/pkg/util/metadata" ) @@ -1159,3 +1166,44 @@ func testConfigFromEnv(t *testing.T, cfg *Config) { t.Skip("No config found in environment") } } + +func TestFetchClusterUID(t *testing.T) { + t.Run("returns kube-system UID", func(t *testing.T) { + const wantUID = "11111111-2222-3333-4444-555555555555" + client := fake.NewSimpleClientset(&v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kube-system", + UID: types.UID(wantUID), + }, + }) + + got := fetchClusterUID(client) + if got != wantUID { + t.Errorf("fetchClusterUID() = %q, want %q", got, wantUID) + } + }) + + t.Run("returns empty when namespace missing", func(t *testing.T) { + client := fake.NewSimpleClientset() + + got := fetchClusterUID(client) + if got != "" { + t.Errorf("fetchClusterUID() = %q, want empty string", got) + } + }) + + t.Run("returns empty on RBAC denial without crashing", func(t *testing.T) { + client := fake.NewSimpleClientset() + client.PrependReactor("get", "namespaces", func(_ k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden( + schema.GroupResource{Resource: "namespaces"}, "kube-system", + errors.New("not allowed"), + ) + }) + + got := fetchClusterUID(client) + if got != "" { + t.Errorf("fetchClusterUID() = %q, want empty string on forbidden error", got) + } + }) +} From ce06ef63d7886075c51c06f4ab393b4e9d182851 Mon Sep 17 00:00:00 2001 From: enginrect Date: Thu, 14 May 2026 08:07:40 +0900 Subject: [PATCH 2/4] chart: bump openstack-cloud-controller-manager to 2.36.1 The previous commit added a "get" on "namespaces" rule to the Helm chart's ClusterRole template. chart-testing requires a version bump on any chart modification. Bumping the patch version since the change is additive and backward-compatible. --- charts/openstack-cloud-controller-manager/Chart.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charts/openstack-cloud-controller-manager/Chart.yaml b/charts/openstack-cloud-controller-manager/Chart.yaml index 49cee5ef22..cb52ebcfdb 100644 --- a/charts/openstack-cloud-controller-manager/Chart.yaml +++ b/charts/openstack-cloud-controller-manager/Chart.yaml @@ -4,7 +4,7 @@ description: Openstack Cloud Controller Manager Helm Chart icon: https://object-storage-ca-ymq-1.vexxhost.net/swift/v1/6e4619c416ff4bd19e1c087f27a43eea/www-images-prod/openstack-logo/OpenStack-Logo-Vertical.png home: https://github.com/kubernetes/cloud-provider-openstack name: openstack-cloud-controller-manager -version: 2.36.0 +version: 2.36.1 maintainers: - name: eumel8 email: f.kloeker@telekom.de From 699f129fbed0c72b3606a571c0fcdd52382ef990 Mon Sep 17 00:00:00 2001 From: enginrect Date: Thu, 14 May 2026 08:07:40 +0900 Subject: [PATCH 3/4] Address review feedback from kayrus - Remove duplicate clusterUID field from the OpenStack struct so the identifier lives only on the LoadBalancer struct that actually uses it (better cohesion). - Drop fetchClusterUID() out of Initialize() and call it lazily inside the LoadBalancer() factory instead. Clusters that disable LB now skip the kube-system namespace lookup entirely, and the change touches only the LB construction path. - Remove the unused eventLBStolen constant. It was a leftover from an earlier draft that intended to emit a Warning event from getLoadbalancerByName(); plumbing the eventRecorder + *v1.Service into that free-standing function felt out of scope, so the emission was dropped but the constant was left behind. --- pkg/openstack/loadbalancer.go | 6 ------ pkg/openstack/openstack.go | 9 ++------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/pkg/openstack/loadbalancer.go b/pkg/openstack/loadbalancer.go index ab8ed0859d..62c3093f24 100644 --- a/pkg/openstack/loadbalancer.go +++ b/pkg/openstack/loadbalancer.go @@ -112,12 +112,6 @@ const ( // OCCM to disambiguate load balancers when multiple Kubernetes clusters // share the same OpenStack project and a service name happens to collide. clusterIDTagPrefix = "kube_cluster_id_" - - // eventLBStolen is emitted when getLoadbalancerByName finds a load - // balancer with a matching name that belongs to a different Kubernetes - // cluster (different cluster-id tag). The lookup is treated as NotFound - // so OCCM creates a new load balancer instead of stealing an existing one. - eventLBStolen = "LoadBalancerNameCollision" ) // clusterIDTag formats the Octavia load balancer tag carrying the cluster diff --git a/pkg/openstack/openstack.go b/pkg/openstack/openstack.go index 726ec6b627..de3132f6b7 100644 --- a/pkg/openstack/openstack.go +++ b/pkg/openstack/openstack.go @@ -167,11 +167,6 @@ type OpenStack struct { kclient kubernetes.Interface nodeInformer coreinformers.NodeInformer nodeInformerHasSynced func() bool - // clusterUID is the kube-system namespace UID, used as a stable cluster - // identifier on OpenStack load balancer tags. May be empty if the lookup - // failed or RBAC does not allow it; in that case OCCM falls back to the - // legacy name-based load balancer identification. - clusterUID string eventBroadcaster record.EventBroadcaster eventRecorder record.EventRecorder @@ -211,7 +206,6 @@ func (os *OpenStack) Initialize(clientBuilder cloudprovider.ControllerClientBuil os.eventBroadcaster = record.NewBroadcaster() os.eventBroadcaster.StartRecordingToSink(&v1core.EventSinkImpl{Interface: os.kclient.CoreV1().Events("")}) os.eventRecorder = os.eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "cloud-provider-openstack"}) - os.clusterUID = fetchClusterUID(clientset) } // fetchClusterUID returns the UID of the kube-system namespace, which is a @@ -405,7 +399,8 @@ func (os *OpenStack) LoadBalancer() (cloudprovider.LoadBalancer, bool) { klog.V(1).Info("Claiming to support LoadBalancer") - return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, os.clusterUID}}, true + clusterUID := fetchClusterUID(os.kclient) + return &LbaasV2{LoadBalancer{secret, network, lb, os.lbOpts, os.kclient, os.eventRecorder, clusterUID}}, true } // Zones indicates that we support zones From efe341dde1bbc7a4492b536fe6e709f4b71a9203 Mon Sep 17 00:00:00 2001 From: enginrect Date: Thu, 14 May 2026 08:07:40 +0900 Subject: [PATCH 4/4] test: replace deprecated fake.NewSimpleClientset with fake.NewClientset The pull-cloud-provider-openstack-check prow job runs golangci-lint v2.3.1 with staticcheck enabled, which flags fake.NewSimpleClientset as SA1019 (deprecated). TestFetchClusterUID, added earlier in this PR, used the deprecated function. Swap it for fake.NewClientset; the signature is identical (objects ...runtime.Object) and the unit tests still pass. --- pkg/openstack/openstack_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/openstack/openstack_test.go b/pkg/openstack/openstack_test.go index d506b85f50..de74bdcfd0 100644 --- a/pkg/openstack/openstack_test.go +++ b/pkg/openstack/openstack_test.go @@ -1170,7 +1170,7 @@ func testConfigFromEnv(t *testing.T, cfg *Config) { func TestFetchClusterUID(t *testing.T) { t.Run("returns kube-system UID", func(t *testing.T) { const wantUID = "11111111-2222-3333-4444-555555555555" - client := fake.NewSimpleClientset(&v1.Namespace{ + client := fake.NewClientset(&v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "kube-system", UID: types.UID(wantUID), @@ -1184,7 +1184,7 @@ func TestFetchClusterUID(t *testing.T) { }) t.Run("returns empty when namespace missing", func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewClientset() got := fetchClusterUID(client) if got != "" { @@ -1193,7 +1193,7 @@ func TestFetchClusterUID(t *testing.T) { }) t.Run("returns empty on RBAC denial without crashing", func(t *testing.T) { - client := fake.NewSimpleClientset() + client := fake.NewClientset() client.PrependReactor("get", "namespaces", func(_ k8stesting.Action) (bool, runtime.Object, error) { return true, nil, apierrors.NewForbidden( schema.GroupResource{Resource: "namespaces"}, "kube-system",