From ab636219c3b8774e8f2fe3c7f5b1ed504888ce72 Mon Sep 17 00:00:00 2001 From: Enxebre Date: Tue, 17 Mar 2020 12:11:18 +0100 Subject: [PATCH] UPSTREAM: : Set default annotations, and cluster name label to use machine.openshift.io This change updates the default annotations and cluster name label to use the openshift specific values. Also updates the unit tests to incorporate the changed api group and adds the capi group environment variable awareness to the tests. Co-authored-by: Michael McCune --- .../clusterapi/clusterapi_controller_test.go | 64 +++++++++++++------ .../clusterapi/clusterapi_nodegroup.go | 8 ++- .../clusterapi/clusterapi_nodegroup_test.go | 10 +-- .../clusterapi/clusterapi_utils.go | 3 + 4 files changed, 61 insertions(+), 24 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 158c5b78e75e..11ad941c8916 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -73,6 +73,14 @@ const customCAPIGroup = "custom.x-k8s.io" const fifteenSecondDuration = time.Second * 15 func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machineController, testControllerShutdownFunc) { + // we need to set the environment variable for the CAPI Group to represent the OpenShift specific value + // UNLESS, the specific test has already set it, in which case we should ignore. + if _, found := os.LookupEnv(CAPIGroupEnvVar); !found { + if err := os.Setenv(CAPIGroupEnvVar, "machine.openshift.io"); err != nil { + t.Fatalf("failed to set CAPI_GROUP environment variable: %v", err) + } + } + t.Helper() nodeObjects := make([]runtime.Object, 0) @@ -97,21 +105,38 @@ func mustCreateTestController(t *testing.T, 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: "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: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinedeployments"}: "kindList", - {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machines"}: "kindList", - {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinesets"}: "kindList", + {Group: "machine.openshift.io", Version: "v1beta1", Resource: "machinedeployments"}: "kindList", + {Group: "machine.openshift.io", Version: "v1beta1", Resource: "machines"}: "kindList", + {Group: "machine.openshift.io", Version: "v1beta1", Resource: "machinesets"}: "kindList", + {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: "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: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinedeployments"}: "kindList", + {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machines"}: "kindList", + {Group: "custom.x-k8s.io", Version: "v1beta1", Resource: "machinesets"}: "kindList", }, machineObjects..., ) discoveryClient := &fakediscovery.FakeDiscovery{ Fake: &clientgotesting.Fake{ Resources: []*metav1.APIResourceList{ + { + GroupVersion: fmt.Sprintf("%s/v1beta1", "machine.openshift.io"), + APIResources: []metav1.APIResource{ + { + Name: resourceNameMachineDeployment, + }, + { + Name: resourceNameMachineSet, + }, + { + Name: resourceNameMachine, + }, + }, + }, { GroupVersion: fmt.Sprintf("%s/v1beta1", customCAPIGroup), APIResources: []metav1.APIResource{ @@ -127,7 +152,7 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin }, }, { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta1", defaultCAPIGroup), APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, @@ -166,7 +191,7 @@ func mustCreateTestController(t *testing.T, testConfigs ...*testConfig) (*machin gvr := schema.GroupVersionResource{ Group: action.GetResource().Group, - Version: "v1alpha3", + Version: "v1beta1", Resource: resource, } @@ -307,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": "machine.openshift.io/v1beta1", "metadata": map[string]interface{}{ "name": spec.machineSetName, "namespace": spec.namespace, @@ -336,7 +361,7 @@ func createTestConfigs(specs ...testSpec) []*testConfig { config.machineDeployment = &unstructured.Unstructured{ Object: map[string]interface{}{ "kind": machineDeploymentKind, - "apiVersion": "cluster.x-k8s.io/v1alpha3", + "apiVersion": "machine.openshift.io/v1beta1", "metadata": map[string]interface{}{ "name": spec.machineDeploymentName, "namespace": spec.namespace, @@ -403,7 +428,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": "machine.openshift.io/v1beta1", "metadata": map[string]interface{}{ "name": fmt.Sprintf("%s-%s-machine-%d", namespace, owner.Name, i), "namespace": namespace, @@ -586,6 +611,7 @@ func TestControllerFindMachine(t *testing.T) { nodeGroupMinSizeAnnotationKey: "1", nodeGroupMaxSizeAnnotationKey: "10", }) + if tc.name == "" { tc.name = testConfig.machines[0].GetName() } @@ -1433,7 +1459,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { { description: "find version for default API group", APIGroup: defaultCAPIGroup, - preferredVersion: "v1alpha3", + preferredVersion: "v1beta1", error: false, }, { @@ -1457,7 +1483,7 @@ func TestGetAPIGroupPreferredVersion(t *testing.T) { GroupVersion: fmt.Sprintf("%s/v1beta1", customCAPIGroup), }, { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta1", defaultCAPIGroup), }, }, }, @@ -1486,14 +1512,14 @@ func TestGroupVersionHasResource(t *testing.T) { { description: "true when it finds resource", resourceName: resourceNameMachineDeployment, - APIGroup: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + APIGroup: fmt.Sprintf("%s/v1beta1", 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/v1beta1", defaultCAPIGroup), expected: false, error: false, }, @@ -1510,7 +1536,7 @@ func TestGroupVersionHasResource(t *testing.T) { Fake: &clientgotesting.Fake{ Resources: []*metav1.APIResourceList{ { - GroupVersion: fmt.Sprintf("%s/v1alpha3", defaultCAPIGroup), + GroupVersion: fmt.Sprintf("%s/v1beta1", defaultCAPIGroup), APIResources: []metav1.APIResource{ { Name: resourceNameMachineDeployment, diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index 63f8299570fb..ed7d091a79eb 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -28,7 +28,13 @@ import ( ) const ( - debugFormat = "%s (min: %d, max: %d, replicas: %d)" + // deprecatedMachineDeleteAnnotationKey should not be removed until minimum cluster-api support is v1alpha3 + deprecatedMachineDeleteAnnotationKey = "cluster.k8s.io/delete-machine" + // TODO: determine what currently relies on deprecatedMachineAnnotationKey to determine when it can be removed + deprecatedMachineAnnotationKey = "cluster.k8s.io/machine" + machineDeleteAnnotationKey = "machine.openshift.io/cluster-api-delete-machine" + machineAnnotationKey = "machine.openshift.io/machine" + debugFormat = "%s (min: %d, max: %d, replicas: %d)" ) type nodegroup struct { diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index f78ad247370e..74ee0ca32ba2 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -315,10 +315,11 @@ func TestNodeGroupIncreaseSizeErrors(t *testing.T) { func TestNodeGroupIncreaseSize(t *testing.T) { type testCase struct { - description string - delta int - initial int32 - expected int32 + description string + delta int + initial int32 + initialStatus int32 + expected int32 } test := func(t *testing.T, tc *testCase, testConfig *testConfig) { @@ -392,6 +393,7 @@ func TestNodeGroupDecreaseTargetSize(t *testing.T) { description string delta int initial int32 + initialStatus int32 targetSizeIncrement int32 expected int32 expectedError bool diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go index 231ffc98bebe..7f42dbcce287 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go @@ -29,6 +29,9 @@ import ( const ( deprecatedNodeGroupMinSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-min-size" deprecatedNodeGroupMaxSizeAnnotationKey = "cluster.k8s.io/cluster-api-autoscaler-node-group-max-size" + nodeGroupMinSizeAnnotationKey = "machine.openshift.io/cluster-api-autoscaler-node-group-min-size" + nodeGroupMaxSizeAnnotationKey = "machine.openshift.io/cluster-api-autoscaler-node-group-max-size" + clusterNameLabel = "machine.openshift.io/cluster-name" deprecatedClusterNameLabel = "cluster.k8s.io/cluster-name" )