From 67ac39bff2247eaf8cd515842673d2c3ea449c77 Mon Sep 17 00:00:00 2001 From: vikaschoudhary16 Date: Thu, 21 Mar 2019 15:11:27 +0530 Subject: [PATCH] Drop unused labels from installer machineSets https://jira.coreos.com/browse/CLOUD-407 --- examples/machine-set.yaml | 8 +++---- examples/machine-with-filters.yaml | 6 +++-- examples/machine-with-user-data.yaml | 6 +++-- examples/machine.yaml | 6 +++-- examples/master-machine.yaml | 6 +++-- pkg/actuators/machine/actuator.go | 6 ++++- pkg/actuators/machine/actuator_test.go | 2 +- pkg/actuators/machine/stubs.go | 10 +++++--- pkg/actuators/machine/utils.go | 23 +++++++++++++++---- .../v1beta1/awsmachineproviderconfig_types.go | 4 +--- 10 files changed, 53 insertions(+), 24 deletions(-) diff --git a/examples/machine-set.yaml b/examples/machine-set.yaml index 75f167b2ac..6e4cdad8a5 100644 --- a/examples/machine-set.yaml +++ b/examples/machine-set.yaml @@ -6,8 +6,6 @@ metadata: namespace: test labels: machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: infra - machine.openshift.io/cluster-api-machine-type: master spec: replicas: 2 selector: @@ -19,9 +17,11 @@ spec: labels: machine.openshift.io/cluster-api-machineset: test-master machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: infra - machine.openshift.io/cluster-api-machine-type: master spec: + metadata: + labels: + node-role.kubernetes.io/master: "" + node-role.kubernetes.io/infra: "" providerSpec: value: apiVersion: awsproviderconfig.k8s.io/v1alpha1 diff --git a/examples/machine-with-filters.yaml b/examples/machine-with-filters.yaml index 3263ee89c3..921a24885f 100644 --- a/examples/machine-with-filters.yaml +++ b/examples/machine-with-filters.yaml @@ -7,9 +7,11 @@ metadata: generateName: vs-worker- labels: machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: compute - machine.openshift.io/cluster-api-machine-type: worker spec: + metadata: + labels: + node-role.kubernetes.io/compute: "" + node-role.kubernetes.io/worker: "" providerSpec: value: apiVersion: awsproviderconfig.k8s.io/v1alpha1 diff --git a/examples/machine-with-user-data.yaml b/examples/machine-with-user-data.yaml index 102e4283e6..aad8f80333 100644 --- a/examples/machine-with-user-data.yaml +++ b/examples/machine-with-user-data.yaml @@ -7,9 +7,11 @@ metadata: generateName: vs-master- labels: machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: infra - machine.openshift.io/cluster-api-machine-type: master spec: + metadata: + labels: + node-role.kubernetes.io/master: "" + node-role.kubernetes.io/infra: "" providerSpec: value: apiVersion: awsproviderconfig.k8s.io/v1alpha1 diff --git a/examples/machine.yaml b/examples/machine.yaml index 51943547f4..9c32d1c349 100644 --- a/examples/machine.yaml +++ b/examples/machine.yaml @@ -7,9 +7,11 @@ metadata: generateName: vs-master- labels: machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: infra - machine.openshift.io/cluster-api-machine-type: master spec: + metadata: + labels: + node-role.kuberntes.io/master: "" + node-role.kuberntes.io/infra: "" providerSpec: value: apiVersion: awsproviderconfig.k8s.io/v1alpha1 diff --git a/examples/master-machine.yaml b/examples/master-machine.yaml index b03bf1001a..2c91e3a7a5 100644 --- a/examples/master-machine.yaml +++ b/examples/master-machine.yaml @@ -7,9 +7,11 @@ metadata: generateName: vs-master- labels: machine.openshift.io/cluster-api-cluster: tb-asg-35 - machine.openshift.io/cluster-api-machine-role: infra - machine.openshift.io/cluster-api-machine-type: master spec: + metadata: + labels: + node-role.kubernetes.io/master: "" + node-role.kubernetes.io/infra: "" providerSpec: value: apiVersion: awsproviderconfig.k8s.io/v1alpha1 diff --git a/pkg/actuators/machine/actuator.go b/pkg/actuators/machine/actuator.go index 7b8079c30b..7ff663900a 100644 --- a/pkg/actuators/machine/actuator.go +++ b/pkg/actuators/machine/actuator.go @@ -191,7 +191,11 @@ func (a *Actuator) CreateMachine(cluster *machinev1.Cluster, machine *machinev1. } // We explicitly do NOT want to remove stopped masters. - if !isMaster(machine) { + isMaster, err := a.isMaster(machine) + if err != nil { + return nil, a.handleMachineError(machine, apierrors.CreateMachine("error determining if machine is master: %v", err), createEventAction) + } + if !isMaster { // Prevent having a lot of stopped nodes sitting around. err = removeStoppedMachine(machine, awsClient) if err != nil { diff --git a/pkg/actuators/machine/actuator_test.go b/pkg/actuators/machine/actuator_test.go index 9e737a5e97..57629abe16 100644 --- a/pkg/actuators/machine/actuator_test.go +++ b/pkg/actuators/machine/actuator_test.go @@ -56,7 +56,7 @@ func TestMachineEvents(t *testing.T) { machineInvalidProviderConfig.Spec.ProviderSpec.ValueFrom = nil workerMachine := machine.DeepCopy() - workerMachine.Labels[providerconfigv1.MachineTypeLabel] = "worker" + workerMachine.Spec.Labels["node-role.kubernetes.io/worker"] = "" cases := []struct { name string diff --git a/pkg/actuators/machine/stubs.go b/pkg/actuators/machine/stubs.go index 90d279cef9..a8f6cc27fa 100644 --- a/pkg/actuators/machine/stubs.go +++ b/pkg/actuators/machine/stubs.go @@ -114,9 +114,7 @@ func stubMachine() (*machinev1.Machine, error) { Name: "aws-actuator-testing-machine", Namespace: defaultNamespace, Labels: map[string]string{ - providerconfigv1.ClusterIDLabel: clusterID, - providerconfigv1.MachineRoleLabel: "infra", - providerconfigv1.MachineTypeLabel: "master", + providerconfigv1.ClusterIDLabel: clusterID, }, Annotations: map[string]string{ // skip node draining since it's not mocked @@ -125,6 +123,12 @@ func stubMachine() (*machinev1.Machine, error) { }, Spec: machinev1.MachineSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "node-role.kubernetes.io/master": "", + "node-role.kubernetes.io/infra": "", + }, + }, ProviderSpec: *providerSpec, }, } diff --git a/pkg/actuators/machine/utils.go b/pkg/actuators/machine/utils.go index de859dfe8e..9fd7cc71df 100644 --- a/pkg/actuators/machine/utils.go +++ b/pkg/actuators/machine/utils.go @@ -188,11 +188,26 @@ func providerConfigFromMachine(client client.Client, machine *machinev1.Machine, } // isMaster returns true if the machine is part of a cluster's control plane -func isMaster(machine *machinev1.Machine) bool { - if machineType, exists := machine.ObjectMeta.Labels[providerconfigv1.MachineTypeLabel]; exists && machineType == "master" { - return true +func (a *Actuator) isMaster(machine *machinev1.Machine) (bool, error) { + if machine.Status.NodeRef == nil { + glog.Errorf("NodeRef not found in machine %s", machine.Name) + return false, nil } - return false + node := &corev1.Node{} + nodeKey := types.NamespacedName{ + Namespace: machine.Status.NodeRef.Namespace, + Name: machine.Status.NodeRef.Name, + } + + err := a.client.Get(context.Background(), nodeKey, node) + if err != nil { + return false, fmt.Errorf("failed to get node from machine %s", machine.Name) + } + + if _, exists := node.Labels["node-role.kubernetes.io/master"]; exists { + return true, nil + } + return false, nil } // updateConditionCheck tests whether a condition should be updated from the diff --git a/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go b/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go index 99de171a53..1d4741fe86 100644 --- a/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go +++ b/pkg/apis/awsproviderconfig/v1beta1/awsmachineproviderconfig_types.go @@ -25,9 +25,7 @@ import ( const ( // ClusterIDLabel is the label that a machineset must have to identify the // cluster to which it belongs. - ClusterIDLabel = "machine.openshift.io/cluster-api-cluster" - MachineRoleLabel = "machine.openshift.io/cluster-api-machine-role" - MachineTypeLabel = "machine.openshift.io/cluster-api-machine-type" + ClusterIDLabel = "machine.openshift.io/cluster-api-cluster" ) // EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!