From f4c2fdebeb092bfea4aa709c0624296085c12b47 Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Wed, 6 Aug 2025 20:48:13 +0800 Subject: [PATCH 1/3] Add process with apiGroup in capi provider --- .../clusterapi/clusterapi_controller.go | 10 +++++-- .../clusterapi/clusterapi_controller_test.go | 10 +++---- .../clusterapi/clusterapi_unstructured.go | 26 +++++++++++++++---- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go index bf6cca922671..b3b7175dfa95 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go @@ -82,6 +82,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 @@ -457,7 +458,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) } @@ -561,6 +562,7 @@ func newMachineController( nodeInformer: nodeInformer, managementClient: managementClient, managementScaleClient: managementScaleClient, + managementDiscoveryClient: managementDiscoveryClient, machineSetResource: gvrMachineSet, machinePoolResource: gvrMachinePool, machinePoolsAvailable: machinePoolsAvailable, @@ -586,11 +588,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 6823ba0a07fc..d76ef40ec8ad 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/types" "math/rand" "path" "reflect" @@ -36,6 +35,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" @@ -416,9 +416,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", }, }, }, @@ -1595,7 +1595,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 6a1b0e085e6a..5d6dd4046465 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -380,17 +380,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 21ca04ae26caf1be335aca4a214cbe040792bcba Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Fri, 8 Aug 2025 11:47:27 +0800 Subject: [PATCH 2/3] 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 d76ef40ec8ad..5515c0e006b3 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -107,13 +107,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", @@ -151,7 +151,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, @@ -191,7 +191,7 @@ func mustCreateTestController(t testing.TB, testConfigs ...*testConfig) (*machin gvr := schema.GroupVersionResource{ Group: action.GetResource().Group, - Version: "v1alpha3", + Version: "v1beta2", Resource: resource, } @@ -366,7 +366,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, @@ -404,7 +404,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, @@ -506,7 +506,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, @@ -1550,7 +1550,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { { description: "find version for default API group", APIGroup: defaultCAPIGroup, - preferredVersion: "v1alpha3", + preferredVersion: "v1beta2", envVar: "", error: false, }, @@ -1584,7 +1584,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), @@ -1617,14 +1617,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, }, @@ -1641,7 +1641,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 1ca5f44ff5dd628c82b530375a5264f93d6a3b2f Mon Sep 17 00:00:00 2001 From: Jun Wang Date: Fri, 8 Aug 2025 14:43:00 +0800 Subject: [PATCH 3/3] 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 5d6dd4046465..f755485e8da8 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -375,6 +375,9 @@ func (r unstructuredScalableResource) InstanceDRADriver() string { } 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 @@ -393,20 +396,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))