From 0a2eddacf5fb3c177fc4fb92ec0877ecbc369171 Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Wed, 6 Aug 2025 20:48:13 +0800 Subject: [PATCH 1/4] Add process with apiGroup in capi provider --- .../clusterapi/clusterapi_controller.go | 10 +++++-- .../clusterapi/clusterapi_controller_test.go | 9 ++++--- .../clusterapi/clusterapi_unstructured.go | 26 +++++++++++++++---- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index 967177cafa64..d0cbbb30fba2 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -81,6 +81,7 @@ type machineController struct { nodeInformer cache.SharedIndexInformer managementClient dynamic.Interface managementScaleClient scale.ScalesGetter + managementDiscoveryClient discovery.DiscoveryInterface machineSetResource schema.GroupVersionResource machineResource schema.GroupVersionResource machinePoolResource schema.GroupVersionResource @@ -422,7 +423,7 @@ func newMachineController( managementInformerFactory := dynamicinformer.NewFilteredDynamicSharedInformerFactory(managementClient, 0, namespaceToWatch(autoDiscoverySpecs), nil) CAPIGroup := getCAPIGroup() - CAPIVersion, err := getAPIGroupPreferredVersion(managementDiscoveryClient, CAPIGroup) + CAPIVersion, err := getCAPIGroupPreferredVersion(managementDiscoveryClient, CAPIGroup) if err != nil { return nil, fmt.Errorf("could not find preferred version for CAPI group %q: %v", CAPIGroup, err) } @@ -526,6 +527,7 @@ func newMachineController( nodeInformer: nodeInformer, managementClient: managementClient, managementScaleClient: managementScaleClient, + managementDiscoveryClient: managementDiscoveryClient, machineSetResource: gvrMachineSet, machinePoolResource: gvrMachinePool, machinePoolsAvailable: machinePoolsAvailable, @@ -551,11 +553,15 @@ func groupVersionHasResource(client discovery.DiscoveryInterface, groupVersion, return false, nil } -func getAPIGroupPreferredVersion(client discovery.DiscoveryInterface, APIGroup string) (string, error) { +func getCAPIGroupPreferredVersion(client discovery.DiscoveryInterface, APIGroup string) (string, error) { if version := os.Getenv(CAPIVersionEnvVar); version != "" { return version, nil } + return getAPIGroupPreferredVersion(client, APIGroup) +} + +func getAPIGroupPreferredVersion(client discovery.DiscoveryInterface, APIGroup string) (string, error) { groupList, err := client.ServerGroups() if err != nil { return "", fmt.Errorf("failed to get ServerGroups: %v", err) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 4e63a7fa6401..9a92a19e9539 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -34,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" fakediscovery "k8s.io/client-go/discovery/fake" "k8s.io/client-go/dynamic" @@ -381,9 +382,9 @@ func createTestConfigs(specs ...testSpec) []*testConfig { "template": map[string]interface{}{ "spec": map[string]interface{}{ "infrastructureRef": map[string]interface{}{ - "apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1", - "kind": machineTemplateKind, - "name": "TestMachineTemplate", + "apiGroup": "infrastructure.cluster.x-k8s.io", + "kind": machineTemplateKind, + "name": "TestMachineTemplate", }, }, }, @@ -1573,7 +1574,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { t.Setenv(CAPIVersionEnvVar, tc.envVar) - version, err := getAPIGroupPreferredVersion(discoveryClient, tc.APIGroup) + version, err := getCAPIGroupPreferredVersion(discoveryClient, tc.APIGroup) if (err != nil) != tc.error { t.Errorf("expected to have error: %t. Had an error: %t", tc.error, err != nil) } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go index f374dc7789f4..c8ec755c76f2 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -327,17 +327,33 @@ func (r unstructuredScalableResource) readInfrastructureReferenceResource() (*un return nil, nil } - apiversion, ok := infraref["apiVersion"] - if !ok { - return nil, nil + var apiversion string + + apiGroup, ok := infraref["apiGroup"] + if ok { + if apiversion, err = getAPIGroupPreferredVersion(r.controller.managementDiscoveryClient, apiGroup); err != nil { + klog.V(4).Infof("Unable to read preferred version from api group %s, error: %v", apiGroup, err) + return nil, err + } + apiversion = fmt.Sprintf("%s/%s", apiGroup, apiversion) + } else { + // Fall back to ObjectReference in capi v1beta1 + apiversion, ok = infraref["apiVersion"] + if !ok { + klog.V(4).Info("Missing apiVersion") + return nil, errors.New("Missing apiVersion") + } } + kind, ok := infraref["kind"] if !ok { - return nil, nil + klog.V(4).Info("Missing kind") + return nil, errors.New("Missing kind") } name, ok := infraref["name"] if !ok { - return nil, nil + klog.V(4).Info("Missing name") + return nil, errors.New("Missing name") } // kind needs to be lower case and plural kind = fmt.Sprintf("%ss", strings.ToLower(kind)) From 58ccff1c02ba5c4ffd3296a21bfb911404eee154 Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Fri, 8 Aug 2025 11:47:27 +0800 Subject: [PATCH 2/4] Replace capi v1alpha3 with v1beta2 in test cases --- .../clusterapi/clusterapi_controller_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 9a92a19e9539..50953c1bb5e9 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -106,13 +106,13 @@ func mustCreateTestController(t testing.TB, testConfigs ...*testConfig) (*machin dynamicClientset := fakedynamic.NewSimpleDynamicClientWithCustomListKinds( runtime.NewScheme(), map[schema.GroupVersionResource]string{ - {Group: "cluster.x-k8s.io", Version: "v1alpha3", Resource: "machinedeployments"}: "kindList", - {Group: "cluster.x-k8s.io", Version: "v1alpha3", Resource: "machines"}: "kindList", - {Group: "cluster.x-k8s.io", Version: "v1alpha3", Resource: "machinesets"}: "kindList", - {Group: "cluster.x-k8s.io", Version: "v1alpha3", Resource: "machinepools"}: "kindList", {Group: "cluster.x-k8s.io", Version: "v1beta1", Resource: "machinedeployments"}: "kindList", {Group: "cluster.x-k8s.io", Version: "v1beta1", Resource: "machines"}: "kindList", {Group: "cluster.x-k8s.io", Version: "v1beta1", Resource: "machinesets"}: "kindList", + {Group: "cluster.x-k8s.io", Version: "v1beta2", Resource: "machinedeployments"}: "kindList", + {Group: "cluster.x-k8s.io", Version: "v1beta2", Resource: "machines"}: "kindList", + {Group: "cluster.x-k8s.io", Version: "v1beta2", Resource: "machinesets"}: "kindList", + {Group: "cluster.x-k8s.io", Version: "v1beta2", Resource: "machinepools"}: "kindList", {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinepools"}: "kindList", {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinedeployments"}: "kindList", {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machines"}: "kindList", @@ -150,7 +150,7 @@ func mustCreateTestController(t testing.TB, testConfigs ...*testConfig) (*machin }, }, { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta2", defaultCAPIGroup), APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, @@ -190,7 +190,7 @@ func mustCreateTestController(t testing.TB, testConfigs ...*testConfig) (*machin gvr := schema.GroupVersionResource{ Group: action.GetResource().Group, - Version: "v1alpha3", + Version: "v1beta2", Resource: resource, } @@ -332,7 +332,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { config.machineSet = &unstructured.Unstructured{ Object: map[string]interface{}{ "kind": machineSetKind, - "apiVersion": "cluster.x-k8s.io/v1alpha3", + "apiVersion": "cluster.x-k8s.io/v1beta2", "metadata": map[string]interface{}{ "name": spec.machineSetName, "namespace": spec.namespace, @@ -370,7 +370,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { config.machineDeployment = &unstructured.Unstructured{ Object: map[string]interface{}{ "kind": machineDeploymentKind, - "apiVersion": "cluster.x-k8s.io/v1alpha3", + "apiVersion": "cluster.x-k8s.io/v1beta2", "metadata": map[string]interface{}{ "name": spec.machineDeploymentName, "namespace": spec.namespace, @@ -472,7 +472,7 @@ func makeLinkedNodeAndMachine(i int, namespace, clusterName string, owner metav1 machine := &unstructured.Unstructured{ Object: map[string]interface{}{ "kind": machineKind, - "apiVersion": "cluster.x-k8s.io/v1alpha3", + "apiVersion": "cluster.x-k8s.io/v1beta2", "metadata": map[string]interface{}{ "name": fmt.Sprintf("%s-%s-machine-%d", namespace, owner.Name, i), "namespace": namespace, @@ -1529,7 +1529,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { { description: "find version for default API group", APIGroup: defaultCAPIGroup, - preferredVersion: "v1alpha3", + preferredVersion: "v1beta2", envVar: "", error: false, }, @@ -1563,7 +1563,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { GroupVersion: fmt.Sprintf("%s/v1beta1", customCAPIGroup), }, { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta2", defaultCAPIGroup), }, { GroupVersion: fmt.Sprintf("%s/%s", customCAPIGroup, customVersion), @@ -1596,14 +1596,14 @@ func TestGroupVersionHasResource(t *testing.T) { { description: "true when it finds resource", resourceName: resourceNameMachineDeployment, - APIGroup: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + APIGroup: fmt.Sprintf("%s/v1beta2", defaultCAPIGroup), expected: true, error: false, }, { description: "false when it does not find resource", resourceName: "resourceDoesNotExist", - APIGroup: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + APIGroup: fmt.Sprintf("%s/v1beta2", defaultCAPIGroup), expected: false, error: false, }, @@ -1620,7 +1620,7 @@ func TestGroupVersionHasResource(t *testing.T) { Fake: &clientgotesting.Fake{ Resources: []*metav1.APIResourceList{ { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta2", defaultCAPIGroup), APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, From 7c46711781dd65c89300641f17e6c6be2fd4303d Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Fri, 8 Aug 2025 14:43:00 +0800 Subject: [PATCH 3/4] Add detailed error messages --- .../clusterapi/clusterapi_unstructured.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go index c8ec755c76f2..6cd37f6e9b6d 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -322,6 +322,9 @@ func (r unstructuredScalableResource) InstanceMaxPodsCapacityAnnotation() (resou } func (r unstructuredScalableResource) readInfrastructureReferenceResource() (*unstructured.Unstructured, error) { + obKind := r.unstructured.GetKind() + obName := r.unstructured.GetName() + infraref, found, err := unstructured.NestedStringMap(r.unstructured.Object, "spec", "template", "spec", "infrastructureRef") if !found || err != nil { return nil, nil @@ -340,20 +343,23 @@ func (r unstructuredScalableResource) readInfrastructureReferenceResource() (*un // Fall back to ObjectReference in capi v1beta1 apiversion, ok = infraref["apiVersion"] if !ok { - klog.V(4).Info("Missing apiVersion") - return nil, errors.New("Missing apiVersion") + info := fmt.Sprintf("Missing apiVersion from %s %s's InfrastructureReference", obKind, obName) + klog.V(4).Info(info) + return nil, errors.New(info) } } kind, ok := infraref["kind"] if !ok { - klog.V(4).Info("Missing kind") - return nil, errors.New("Missing kind") + info := fmt.Sprintf("Missing kind from %s %s's InfrastructureReference", obKind, obName) + klog.V(4).Info(info) + return nil, errors.New(info) } name, ok := infraref["name"] if !ok { - klog.V(4).Info("Missing name") - return nil, errors.New("Missing name") + info := fmt.Sprintf("Missing name from %s %s's InfrastructureReference", obKind, obName) + klog.V(4).Info(info) + return nil, errors.New(info) } // kind needs to be lower case and plural kind = fmt.Sprintf("%ss", strings.ToLower(kind)) From 06f2556ee01286367d64efdc800be3590b026dc0 Mon Sep 17 00:00:00 2001 From: Jack Francis Date: Fri, 12 Sep 2025 09:46:30 -0700 Subject: [PATCH 4/4] remove unused package --- .../cloudprovider/clusterapi/clusterapi_controller_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 50953c1bb5e9..191d93856ce5 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" fakediscovery "k8s.io/client-go/discovery/fake" "k8s.io/client-go/dynamic"