From c6577bbef824f993f60c52e3c5007dc3b972aa5b Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 19 Sep 2022 12:35:42 +0530 Subject: [PATCH 1/3] removing Node field from machine status --- Makefile | 2 +- docs/documents/apis.md | 33 +- docs/todo/outline.md | 97 +-- ...ne.sapcloud.io_alicloudmachineclasses.yaml | 11 +- ...machine.sapcloud.io_awsmachineclasses.yaml | 11 +- ...chine.sapcloud.io_azuremachineclasses.yaml | 11 +- ...machine.sapcloud.io_gcpmachineclasses.yaml | 11 +- .../machine.sapcloud.io_machineclasses.yaml | 11 +- ...achine.sapcloud.io_machinedeployments.yaml | 10 +- .../crds/machine.sapcloud.io_machines.yaml | 12 +- .../crds/machine.sapcloud.io_machinesets.yaml | 10 +- ...e.sapcloud.io_openstackmachineclasses.yaml | 11 +- ...hine.sapcloud.io_packetmachineclasses.yaml | 11 +- pkg/apis/machine/types.go | 8 +- pkg/apis/machine/v1alpha1/machine_types.go | 8 +- .../v1alpha1/zz_generated.conversion.go | 2 - pkg/controller/controller_suite_test.go | 12 +- pkg/controller/deployment_rollback.go | 12 +- pkg/controller/deployment_rollback_test.go | 6 +- pkg/controller/deployment_rolling.go | 24 +- pkg/controller/deployment_rolling_test.go | 48 +- pkg/controller/deployment_test.go | 5 +- pkg/controller/machine.go | 45 +- pkg/controller/machine_test.go | 90 +-- pkg/controller/machine_util.go | 5 +- pkg/controller/machine_util_test.go | 42 +- pkg/controller/secret.go | 2 +- pkg/openapi/openapi_generated.go | 7 - .../controller_suite_test.go | 8 +- .../provider/machinecontroller/machine.go | 35 +- .../machinecontroller/machine_safety_test.go | 6 +- .../machinecontroller/machine_test.go | 701 ++---------------- .../machinecontroller/machine_util.go | 12 +- .../machinecontroller/machine_util_test.go | 128 ++-- 34 files changed, 315 insertions(+), 1132 deletions(-) diff --git a/Makefile b/Makefile index a33f45696..bf579ad8e 100644 --- a/Makefile +++ b/Makefile @@ -163,7 +163,7 @@ ifeq (, $(shell which controller-gen)) CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\ cd $$CONTROLLER_GEN_TMP_DIR ;\ go mod init tmp ;\ - go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.2 ;\ + go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.9.2 ;\ rm -rf $$CONTROLLER_GEN_TMP_DIR ;\ } CONTROLLER_GEN=$(GOBIN)/controller-gen diff --git a/docs/documents/apis.md b/docs/documents/apis.md index cd7066f5e..afb6ea9ed 100644 --- a/docs/documents/apis.md +++ b/docs/documents/apis.md @@ -1,9 +1,4 @@ ---- -title: APIs ---- - ## Specification - ### ProviderSpec Schema

@@ -2793,9 +2788,10 @@ string

AzureLinuxConfiguration is specifies the Linux operating system settings on the virtual machine.

For a list of -supported Linux distributions, see Endorsed Linux distributions on Azure +supported Linux distributions, see Linux on Azure-Endorsed Distributions -

For running non-endorsed distributions, see Information for community supported and non-endorsed distributions.

+

For running non-endorsed distributions, see Information for Non-Endorsed +Distributions.

@@ -3179,8 +3175,9 @@ AzureNetworkInterfaceReference AzureStorageProfile)

-

AzureOSDisk specifies information about the operating system disk used by the virtual machine.

For more -information about disks, see Introduction to Azure managed disks.

+

AzureOSDisk is specifies information about the operating system disk used by the virtual machine.

For more +information about disks, see About disks and VHDs for Azure virtual +machines.

@@ -5747,19 +5744,6 @@ MachineConfiguration - - - - - @@ -5947,7 +5930,7 @@ MachineSpec
-node - - -string - - -

Node string

-
conditions @@ -5927,8 +5911,7 @@ Kubernetes meta/v1.ObjectMeta (Optional)

Standard object’s metadata. -More info: API Conventions - Metadata

+More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata

Refer to the Kubernetes API documentation for the fields of the metadata field. 		(Optional)

Specification of the desired behavior of the machine. -More info: API Conventions - Spec and Status

+More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status



diff --git a/docs/todo/outline.md b/docs/todo/outline.md index 42aa8fcb9..6234c5b71 100644 --- a/docs/todo/outline.md +++ b/docs/todo/outline.md @@ -14,57 +14,58 @@ MCM is a set controllers: ## Questions and refactoring Suggestions ### Refactoring -| Statement | FilePath | Status | -| -- | -- | -- | -| ConcurrentNodeSyncs” bad name - nothing to do with node syncs actually.
If its value is ’10’ then it will start 10 goroutines (workers) per resource type (machine, machinist, machinedeployment, provider-specific-class, node - study the different resource types. | cmd/machine-controller-manager/app/options/options.go | pending | -|LeaderElectionConfiguration is very similar to the one present in “client-go/tools/leaderelection/leaderelection.go” - can we simply used the one in client-go instead of defining again?| pkg/options/types.go - MachineControllerManagerConfiguration | pending | -|Have all userAgents as constant. Right now there is just one. | cmd/app/controllermanager.go | pending | -|Shouldn’t run function be defined on MCMServer struct itself?|cmd/app/controllermanager.go | pending | -| clientcmd.BuildConfigFromFlags fallsback to inClusterConfig which will surely not work as that is not the target. Should it not check and exit early? | cmd/app/controllermanager.go - run Function | pending | -| A more direct way to create an in cluster config is using `k8s.io/client-go/rest` -> rest.InClusterConfig instead of using clientcmd.BuildConfigFromFlags passing empty arguments and depending upon the implementation to fallback to creating a inClusterConfig. If they change the implementation that you get affected. | cmd/app/controllermanager.go - run Function | pending | -|Introduce a method on MCMServer which gets a target KubeConfig and controlKubeConfig or alternatively which creates respective clients.| cmd/app/controllermanager.go - run Function | pending | -|Why can’t we use Kubernetes.NewConfigOrDie also for kubeClientControl?| cmd/app/controllermanager.go - run Function | pending | -| I do not see any benefit of client builders actually. All you need to do is pass in a config and then directly use client-go functions to create a client. | cmd/app/controllermanager.go - run Function | pending | -| Function: getAvailableResources - rename this to getApiServerResources | cmd/app/controllermanager.go | pending | -|Move the method which waits for API server to up and ready to a separate method which returns a discoveryClient when the API server is ready. | cmd/app/controllermanager.go - getAvailableResources function | pending | -| Many methods in client-go used are now deprecated. Switch to the ones that are now recommended to be used instead. | cmd/app/controllermanager.go - startControllers | pending | -| This method needs a general overhaul | cmd/app/controllermanager.go - startControllers | pending | -| If the design is influenced/copied from KCM then its very different. There are different controller structs defined for deployment, replicaset etc which makes the code much more clearer. You can see “kubernetes/cmd/kube-controller-manager/apps.go” and then follow the trail from there. - agreed needs to be changed in future (if time permits) | pkg/controller/controller.go | pending | -| I am not sure why “MachineSetControlInterface”, “RevisionControlInterface”, “MachineControlInterface”, “FakeMachineControl” are defined in this file? | pkg/controller/controller_util.go | pending | -| `IsMachineActive` - combine the first 2 conditions into one with OR. | pkg/controller/controller_util.go | pending | -| Minor change - correct the comment, first word should always be the method name. Currently none of the comments have correct names. | pkg/controller/controller_util.go | pending | -| There are too many deep copies made. What is the need to make another deep copy in this method? You are not really changing anything here. | pkg/controller/deployment.go - updateMachineDeploymentFinalizers | pending | -| Why can't these validations be done as part of a validating webhook? | pkg/controller/machineset.go - reconcileClusterMachineSet | pending | -| Small change to the following `if` condition. `else if` is not required a simple `else` is sufficient. [Code1](#1.1-code1) -| pkg/controller/machineset.go - reconcileClusterMachineSet | pending | -| Why call these `inactiveMachines`, these are live and running and therefore active. | pkg/controller/machineset.go - terminateMachines | pending | + +| Statement | FilePath | Status | +|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------|---------| +| ConcurrentNodeSyncs” bad name - nothing to do with node syncs actually.
If its value is ’10’ then it will start 10 goroutines (workers) per resource type (machine, machinist, machinedeployment, provider-specific-class, node - study the different resource types. | cmd/machine-controller-manager/app/options/options.go | pending | +| LeaderElectionConfiguration is very similar to the one present in “client-go/tools/leaderelection/leaderelection.go” - can we simply used the one in client-go instead of defining again? | pkg/options/types.go - MachineControllerManagerConfiguration | pending | +| Have all userAgents as constant. Right now there is just one. | cmd/app/controllermanager.go | pending | +| Shouldn’t run function be defined on MCMServer struct itself? | cmd/app/controllermanager.go | pending | +| clientcmd.BuildConfigFromFlags fallsback to inClusterConfig which will surely not work as that is not the target. Should it not check and exit early? | cmd/app/controllermanager.go - run Function | pending | +| A more direct way to create an in cluster config is using `k8s.io/client-go/rest` -> rest.InClusterConfig instead of using clientcmd.BuildConfigFromFlags passing empty arguments and depending upon the implementation to fallback to creating a inClusterConfig. If they change the implementation that you get affected. | cmd/app/controllermanager.go - run Function | pending | +| Introduce a method on MCMServer which gets a target KubeConfig and controlKubeConfig or alternatively which creates respective clients. | cmd/app/controllermanager.go - run Function | pending | +| Why can’t we use Kubernetes.NewConfigOrDie also for kubeClientControl? | cmd/app/controllermanager.go - run Function | pending | +| I do not see any benefit of client builders actually. All you need to do is pass in a config and then directly use client-go functions to create a client. | cmd/app/controllermanager.go - run Function | pending | +| Function: getAvailableResources - rename this to getApiServerResources | cmd/app/controllermanager.go | pending | +| Move the method which waits for API server to up and ready to a separate method which returns a discoveryClient when the API server is ready. | cmd/app/controllermanager.go - getAvailableResources function | pending | +| Many methods in client-go used are now deprecated. Switch to the ones that are now recommended to be used instead. | cmd/app/controllermanager.go - startControllers | pending | +| This method needs a general overhaul | cmd/app/controllermanager.go - startControllers | pending | +| If the design is influenced/copied from KCM then its very different. There are different controller structs defined for deployment, replicaset etc which makes the code much more clearer. You can see “kubernetes/cmd/kube-controller-manager/apps.go” and then follow the trail from there. - agreed needs to be changed in future (if time permits) | pkg/controller/controller.go | pending | +| I am not sure why “MachineSetControlInterface”, “RevisionControlInterface”, “MachineControlInterface”, “FakeMachineControl” are defined in this file? | pkg/controller/controller_util.go | pending | +| `IsMachineActive` - combine the first 2 conditions into one with OR. | pkg/controller/controller_util.go | pending | +| Minor change - correct the comment, first word should always be the method name. Currently none of the comments have correct names. | pkg/controller/controller_util.go | pending | +| There are too many deep copies made. What is the need to make another deep copy in this method? You are not really changing anything here. | pkg/controller/deployment.go - updateMachineDeploymentFinalizers | pending | +| Why can't these validations be done as part of a validating webhook? | pkg/controller/machineset.go - reconcileClusterMachineSet | pending | +| Small change to the following `if` condition. `else if` is not required a simple `else` is sufficient. [Code1](#1.1-code1) | | | +| pkg/controller/machineset.go - reconcileClusterMachineSet | pending | | +| Why call these `inactiveMachines`, these are live and running and therefore active. | pkg/controller/machineset.go - terminateMachines | pending | ### Clarification -| Statement | FilePath | Status | -| -- | -- | -- | -| Why are there 2 versions - internal and external versions? | General | pending | -| Safety controller freezes MCM controllers in the following cases:
* Num replicas go beyond a threshold (above the defined replicas)
* Target API service is not reachable
There seems to be an overlap between DWD and MCM Safety controller. In the meltdown scenario why is MCM being added to DWD, you could have used Safety controller for that. | General | pending | -| All machine resources are v1alpha1 - should we not promote it to beta. V1alpha1 has a different semantic and does not give any confidence to the consumers. | cmd/app/controllermanager.go | pending | -| Shouldn’t controller manager use context.Context instead of creating a stop channel? - Check if signals (`os.Interrupt` and `SIGTERM` are handled properly. Do not see code where this is handled currently.) | cmd/app/controllermanager.go | pending | -| What is the rationale behind a timeout of 10s? If the API server is not up, should this not just block as it can anyways not do anything. Also, if there is an error returned then you exit the MCM which does not make much sense actually as it will be started again and you will again do the poll for the API server to come back up. Forcing an exit of MCM will not have any impact on the reachability of the API server in anyway so why exit? | cmd/app/controllermanager.go - getAvailableResources | pending | -| There is a very weird check - availableResources[machineGVR] \|\| availableResources[machineSetGVR] \|\| availableResources[machineDeploymentGVR]
\* Shouldn’t this be conjunction instead of disjunction?
\* What happens if you do not find one or all of these resources?
Currently an error log is printed and nothing else is done. MCM can be used outside gardener context where consumers can directly create MachineClass and Machine and not create MachineSet / Maching Deployment. There is no distinction made between context (gardener or outside-gardener). | cmd/app/controllermanager.go - StartControllers | pending | -| Instead of having an empty select {} to block forever, isn’t it better to wait on the stop channel? | cmd/app/controllermanager.go - StartControllers | pending | -| Do we need provider specific queues and syncs and listers | pkg/controller/controller.go | pending | -| Why are resource types prefixed with “Cluster”? - not sure , check PR | pkg/controller/controller.go | pending | -| When will forgetAfterSuccess be false and why? - as per the current code this is never the case. - Himanshu will check | cmd/app/controllermanager.go - createWorker | pending | -| What is the use of “ExpectationsInterface” and “UIDTrackingContExpectations”?
* All expectations related code should be in its own file “expectations.go” and not in this file. | pkg/controller/controller_util.go | pending | -| Why do we not use lister but directly use the controlMachingClient to get the deployment? Is it because you want to avoid any potential delays caused by update of the local cache held by the informer and accessed by the lister? What is the load on API server due to this? | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| Why is this conversion needed? [code2](#1.2-code2) | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| A deep copy of `machineDeployment` is already passed and within the function another deepCopy is made. Any reason for it? | pkg/controller/deployment.go - addMachineDeploymentFinalizers| pending | -| What is an `Status.ObservedGeneration`?
**Read more about generations and observedGeneration at:
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata
* https://alenkacz.medium.com/kubernetes-operator-best-practices-implementing-observedgeneration-250728868792
Ideally the update to the `ObservedGeneration` should only be made after successful reconciliation and not before. I see that this is just copied from `deployment_controller.go` as is | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| Why and when will a `MachineDeployment` be marked as frozen and when will it be un-frozen? | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| Shoudn't the validation of the machine deployment be done during the creation via a validating webhook instead of allowing it to be stored in etcd and then failing the validation during sync? I saw the checks and these can be done via validation webhook. | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| RollbackTo has been marked as deprecated. What is the replacement? [code3](#1.3-code3) | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | -| What is the max machineSet deletions that you could process in a single run? The reason for asking this question is that for every machineSetDeletion a new goroutine spawned.
* Is the `Delete` call a synchrounous call? Which means it blocks till the machineset deletion is triggered which then also deletes the machines (due to cascade-delete and blockOwnerDeletion= true)?| pkg/controller/deployment.go - terminateMachineSets | pending | -| If there are validation errors or error when creating label selector then a nil is returned. In the worker reconcile loop if the return value is nil then it will remove it from the queue (forget + done). What is the way to see any errors? Typically when we describe a resource the errors are displayed. Will these be displayed when we discribe a `MachineDeployment`? | pkg/controller/deployment.go - reconcileClusterMachineSet | pending | -| If an error is returned by `updateMachineSetStatus` and it is `IsNotFound` error then returning an error will again queue the `MachineSet`. Is this desired as `IsNotFound` indicates the `MachineSet` has been deleted and is no longer there? | pkg/controller/deployment.go - reconcileClusterMachineSet | pending | -| is `machineControl.DeleteMachine` a synchronous operation which will wait till the machine has been deleted? Also where is the `DeletionTimestamp` set on the `Machine`? Will it be automatically done by the API server? | pkg/controller/deployment.go - prepareMachineForDeletion | pending | +| Statement | FilePath | Status | +|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------------------------|---------| +| Why are there 2 versions - internal and external versions? | General | pending | +| Safety controller freezes MCM controllers in the following cases:
* Num replicas go beyond a threshold (above the defined replicas)
* Target API service is not reachable
There seems to be an overlap between DWD and MCM Safety controller. In the meltdown scenario why is MCM being added to DWD, you could have used Safety controller for that. | General | pending | +| All machine resources are v1alpha1 - should we not promote it to beta. V1alpha1 has a different semantic and does not give any confidence to the consumers. | cmd/app/controllermanager.go | pending | +| Shouldn’t controller manager use context.Context instead of creating a stop channel? - Check if signals (`os.Interrupt` and `SIGTERM` are handled properly. Do not see code where this is handled currently.) | cmd/app/controllermanager.go | pending | +| What is the rationale behind a timeout of 10s? If the API server is not up, should this not just block as it can anyways not do anything. Also, if there is an error returned then you exit the MCM which does not make much sense actually as it will be started again and you will again do the poll for the API server to come back up. Forcing an exit of MCM will not have any impact on the reachability of the API server in anyway so why exit? | cmd/app/controllermanager.go - getAvailableResources | pending | +| There is a very weird check - availableResources\[machineGVR\] || availableResources\[machineSetGVR\] || availableResources\[machineDeploymentGVR\]
Shouldn’t this be conjunction instead of disjunction?
\* What happens if you do not find one or all of these resources?
Currently an error log is printed and nothing else is done. MCM can be used outside gardener context where consumers can directly create MachineClass and Machine and not create MachineSet / Maching Deployment. There is no distinction made between context (gardener or outside-gardener).
| cmd/app/controllermanager.go - StartControllers | pending | +| Instead of having an empty select {} to block forever, isn’t it better to wait on the stop channel? | cmd/app/controllermanager.go - StartControllers | pending | +| Do we need provider specific queues and syncs and listers | pkg/controller/controller.go | pending | +| Why are resource types prefixed with “Cluster”? - not sure , check PR | pkg/controller/controller.go | pending | +| When will forgetAfterSuccess be false and why? - as per the current code this is never the case. - Himanshu will check | cmd/app/controllermanager.go - createWorker | pending | +| What is the use of “ExpectationsInterface” and “UIDTrackingContExpectations”?
* All expectations related code should be in its own file “expectations.go” and not in this file. | pkg/controller/controller_util.go | pending | +| Why do we not use lister but directly use the controlMachingClient to get the deployment? Is it because you want to avoid any potential delays caused by update of the local cache held by the informer and accessed by the lister? What is the load on API server due to this? | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| Why is this conversion needed? [code2](#1.2-code2) | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| A deep copy of `machineDeployment` is already passed and within the function another deepCopy is made. Any reason for it? | pkg/controller/deployment.go - addMachineDeploymentFinalizers | pending | +| What is an `Status.ObservedGeneration`?
**Read more about generations and observedGeneration at:
https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#metadata
* https://alenkacz.medium.com/kubernetes-operator-best-practices-implementing-observedgeneration-250728868792
Ideally the update to the `ObservedGeneration` should only be made after successful reconciliation and not before. I see that this is just copied from `deployment_controller.go` as is | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| Why and when will a `MachineDeployment` be marked as frozen and when will it be un-frozen? | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| Shoudn't the validation of the machine deployment be done during the creation via a validating webhook instead of allowing it to be stored in etcd and then failing the validation during sync? I saw the checks and these can be done via validation webhook. | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| RollbackTo has been marked as deprecated. What is the replacement? [code3](#1.3-code3) | pkg/controller/deployment.go - reconcileClusterMachineDeployment | pending | +| What is the max machineSet deletions that you could process in a single run? The reason for asking this question is that for every machineSetDeletion a new goroutine spawned.
* Is the `Delete` call a synchrounous call? Which means it blocks till the machineset deletion is triggered which then also deletes the machines (due to cascade-delete and blockOwnerDeletion= true)? | pkg/controller/deployment.go - terminateMachineSets | pending | +| If there are validation errors or error when creating label selector then a nil is returned. In the worker reconcile loop if the return value is nil then it will remove it from the queue (forget + done). What is the way to see any errors? Typically when we describe a resource the errors are displayed. Will these be displayed when we discribe a `MachineDeployment`? | pkg/controller/deployment.go - reconcileClusterMachineSet | pending | +| If an error is returned by `updateMachineSetStatus` and it is `IsNotFound` error then returning an error will again queue the `MachineSet`. Is this desired as `IsNotFound` indicates the `MachineSet` has been deleted and is no longer there? | pkg/controller/deployment.go - reconcileClusterMachineSet | pending | +| is `machineControl.DeleteMachine` a synchronous operation which will wait till the machine has been deleted? Also where is the `DeletionTimestamp` set on the `Machine`? Will it be automatically done by the API server? | pkg/controller/deployment.go - prepareMachineForDeletion | pending | ### Bugs/Enhancements diff --git a/kubernetes/crds/machine.sapcloud.io_alicloudmachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_alicloudmachineclasses.yaml index 0daf2def9..a8745bf36 100644 --- a/kubernetes/crds/machine.sapcloud.io_alicloudmachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_alicloudmachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: alicloudmachineclasses.machine.sapcloud.io spec: @@ -65,6 +64,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic dataDisks: items: properties: @@ -113,6 +113,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic securityGroupID: type: string spotStrategy: @@ -147,9 +148,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_awsmachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_awsmachineclasses.yaml index 41b3e37f6..3841cdd38 100644 --- a/kubernetes/crds/machine.sapcloud.io_awsmachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_awsmachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: awsmachineclasses.machine.sapcloud.io spec: @@ -151,6 +150,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic ebsOptimized: type: boolean iam: @@ -220,6 +220,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic spotPrice: type: string tags: @@ -231,9 +232,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_azuremachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_azuremachineclasses.yaml index c636b6f52..a4a72ce7a 100644 --- a/kubernetes/crds/machine.sapcloud.io_azuremachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_azuremachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: azuremachineclasses.machine.sapcloud.io spec: @@ -63,6 +62,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic location: type: string properties: @@ -237,6 +237,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic subnetInfo: description: AzureSubnetInfo is the information containing the subnet details @@ -257,9 +258,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_gcpmachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_gcpmachineclasses.yaml index f55408ff8..d7f3a87c5 100644 --- a/kubernetes/crds/machine.sapcloud.io_gcpmachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_gcpmachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: gcpmachineclasses.machine.sapcloud.io spec: @@ -65,6 +64,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic deletionProtection: type: boolean description: @@ -162,6 +162,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic serviceAccounts: items: description: GCPServiceAccount describes service accounts for GCP. @@ -196,9 +197,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_machineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_machineclasses.yaml index 6e0678f55..eb0a05b33 100644 --- a/kubernetes/crds/machine.sapcloud.io_machineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: machineclasses.machine.sapcloud.io spec: @@ -42,6 +41,7 @@ spec: must be unique. type: string type: object + x-kubernetes-map-type: atomic kind: description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client @@ -100,14 +100,9 @@ spec: must be unique. type: string type: object + x-kubernetes-map-type: atomic required: - providerSpec type: object served: true storage: true -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml index 09af42a8a..8f6501d1a 100644 --- a/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machinedeployments.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: machinedeployments.machine.sapcloud.io spec: @@ -148,6 +147,7 @@ spec: are ANDed. type: object type: object + x-kubernetes-map-type: atomic strategy: description: The MachineDeployment strategy to use to replace existing machines with new ones. @@ -488,9 +488,3 @@ spec: specReplicasPath: .spec.replicas statusReplicasPath: .status.replicas status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_machines.yaml b/kubernetes/crds/machine.sapcloud.io_machines.yaml index 7d82f0479..d4664ed43 100644 --- a/kubernetes/crds/machine.sapcloud.io_machines.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machines.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: machines.machine.sapcloud.io spec: @@ -269,18 +268,9 @@ spec: description: Type of operation type: string type: object - node: - description: Node string - type: string type: object type: object served: true storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml index bc62ed2d0..312bfa933 100644 --- a/kubernetes/crds/machine.sapcloud.io_machinesets.yaml +++ b/kubernetes/crds/machine.sapcloud.io_machinesets.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: machinesets.machine.sapcloud.io spec: @@ -121,6 +120,7 @@ spec: are ANDed. type: object type: object + x-kubernetes-map-type: atomic template: description: MachineTemplateSpec describes the data a machine should have when created from a template @@ -410,9 +410,3 @@ spec: specReplicasPath: .spec.replicas statusReplicasPath: .status.replicas status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_openstackmachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_openstackmachineclasses.yaml index faaa99ab5..34c67dfd1 100644 --- a/kubernetes/crds/machine.sapcloud.io_openstackmachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_openstackmachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: openstackmachineclasses.machine.sapcloud.io spec: @@ -68,6 +67,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic flavorName: type: string imageID: @@ -108,6 +108,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic securityGroups: items: type: string @@ -137,9 +138,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/kubernetes/crds/machine.sapcloud.io_packetmachineclasses.yaml b/kubernetes/crds/machine.sapcloud.io_packetmachineclasses.yaml index 2c2565225..4bf6ef56f 100644 --- a/kubernetes/crds/machine.sapcloud.io_packetmachineclasses.yaml +++ b/kubernetes/crds/machine.sapcloud.io_packetmachineclasses.yaml @@ -1,10 +1,9 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.6.2 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: packetmachineclasses.machine.sapcloud.io spec: @@ -60,6 +59,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic facility: items: type: string @@ -81,6 +81,7 @@ spec: name must be unique. type: string type: object + x-kubernetes-map-type: atomic sshKeys: items: type: string @@ -102,9 +103,3 @@ spec: served: true storage: true subresources: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/pkg/apis/machine/types.go b/pkg/apis/machine/types.go index 2b2a817fe..57c1a3c80 100644 --- a/pkg/apis/machine/types.go +++ b/pkg/apis/machine/types.go @@ -33,6 +33,11 @@ import ( // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object +// This is the valid key for machine node label +const ( + MachineNodeLabelKey string = "node" +) + // Machine TODO type Machine struct { // ObjectMeta for machine object @@ -167,9 +172,6 @@ type CurrentStatus struct { // MachineStatus holds the most recently observed status of Machine. type MachineStatus struct { - // Node string - Node string - // Conditions of this machine, same as node Conditions []corev1.NodeCondition diff --git a/pkg/apis/machine/v1alpha1/machine_types.go b/pkg/apis/machine/v1alpha1/machine_types.go index 4628562ec..fbce5eca3 100644 --- a/pkg/apis/machine/v1alpha1/machine_types.go +++ b/pkg/apis/machine/v1alpha1/machine_types.go @@ -35,6 +35,11 @@ import ( // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.\nPopulated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata" // +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.metadata.labels.node`,description="Node backing the machine object" +// This is the valid key for machine node label +const ( + MachineNodeLabelKey string = "node" +) + // Machine is the representation of a physical or virtual machine. type Machine struct { // ObjectMeta for machine object @@ -95,9 +100,6 @@ type NodeTemplateSpec struct { // MachineStatus holds the most recently observed status of Machine. type MachineStatus struct { - // Node string - Node string `json:"node,omitempty"` - // Conditions of this machine, same as node Conditions []corev1.NodeCondition `json:"conditions,omitempty"` diff --git a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go index c7f8746e3..c1d4a9a8a 100644 --- a/pkg/apis/machine/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/machine/v1alpha1/zz_generated.conversion.go @@ -2547,7 +2547,6 @@ func Convert_machine_MachineSpec_To_v1alpha1_MachineSpec(in *machine.MachineSpec } func autoConvert_v1alpha1_MachineStatus_To_machine_MachineStatus(in *MachineStatus, out *machine.MachineStatus, s conversion.Scope) error { - out.Node = in.Node out.Conditions = *(*[]v1.NodeCondition)(unsafe.Pointer(&in.Conditions)) if err := Convert_v1alpha1_LastOperation_To_machine_LastOperation(&in.LastOperation, &out.LastOperation, s); err != nil { return err @@ -2565,7 +2564,6 @@ func Convert_v1alpha1_MachineStatus_To_machine_MachineStatus(in *MachineStatus, } func autoConvert_machine_MachineStatus_To_v1alpha1_MachineStatus(in *machine.MachineStatus, out *MachineStatus, s conversion.Scope) error { - out.Node = in.Node out.Conditions = *(*[]v1.NodeCondition)(unsafe.Pointer(&in.Conditions)) if err := Convert_machine_LastOperation_To_v1alpha1_LastOperation(&in.LastOperation, &out.LastOperation, s); err != nil { return err diff --git a/pkg/controller/controller_suite_test.go b/pkg/controller/controller_suite_test.go index 9ae1ecba2..6bdf813ba 100644 --- a/pkg/controller/controller_suite_test.go +++ b/pkg/controller/controller_suite_test.go @@ -340,7 +340,7 @@ func newMachines( } if statusTemplate != nil { - m.Status = *newMachineStatus(statusTemplate, i) + m.Status = *newMachineStatus(statusTemplate) } if owner != nil { @@ -410,18 +410,12 @@ func newMachineSpec(specTemplate *v1alpha1.MachineSpec, index int) *v1alpha1.Mac return r } -func newMachineStatus(statusTemplate *v1alpha1.MachineStatus, index int) *v1alpha1.MachineStatus { +func newMachineStatus(statusTemplate *v1alpha1.MachineStatus) *v1alpha1.MachineStatus { if statusTemplate == nil { return &v1alpha1.MachineStatus{} } - r := statusTemplate.DeepCopy() - if r.Node == "" { - return r - } - - r.Node = fmt.Sprintf("%s-%d", r.Node, index) - return r + return statusTemplate.DeepCopy() } func newSecretReference(meta *metav1.ObjectMeta, index int) *corev1.SecretReference { diff --git a/pkg/controller/deployment_rollback.go b/pkg/controller/deployment_rollback.go index 7d4db3a04..cf477bae3 100644 --- a/pkg/controller/deployment_rollback.go +++ b/pkg/controller/deployment_rollback.go @@ -172,24 +172,24 @@ func (dc *controller) removeTaintNodesBackingMachineSet(ctx context.Context, mac // Iterate through all machines and remove the PreferNoSchedule taint // to avoid scheduling on older machines for _, machine := range filteredMachines { - if machine.Status.Node != "" { - node, err := dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Status.Node, metav1.GetOptions{}) + if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { + node, err := dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) if err != nil { - klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Status.Node, err) + klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) continue } err = nodeops.RemoveTaintOffNode( ctx, dc.targetCoreClient, - machine.Status.Node, + machine.Labels[v1alpha1.MachineNodeLabelKey], node, taint, ) if err != nil { - klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Status.Node, err) + klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) } - node, err = dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Status.Node, metav1.GetOptions{}) + node, err = dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) } } diff --git a/pkg/controller/deployment_rollback_test.go b/pkg/controller/deployment_rollback_test.go index 90a7a3c6c..a41a4aecf 100644 --- a/pkg/controller/deployment_rollback_test.go +++ b/pkg/controller/deployment_rollback_test.go @@ -131,11 +131,9 @@ var _ = Describe("deployment_rollback", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, diff --git a/pkg/controller/deployment_rolling.go b/pkg/controller/deployment_rolling.go index 18232ff70..b1918fecc 100644 --- a/pkg/controller/deployment_rolling.go +++ b/pkg/controller/deployment_rolling.go @@ -322,15 +322,15 @@ func (dc *controller) taintNodesBackingMachineSets(ctx context.Context, MachineS // Iterate through all machines and place the PreferNoSchedule taint // to avoid scheduling on older machines for _, machine := range filteredMachines { - if machine.Status.Node != "" { + if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { err = nodeops.AddOrUpdateTaintOnNode( ctx, dc.targetCoreClient, - machine.Status.Node, + machine.Labels[v1alpha1.MachineNodeLabelKey], taint, ) if err != nil { - klog.Warningf("Node tainting failed for node: %s, %s", machine.Status.Node, err) + klog.Warningf("Node tainting failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) } } } @@ -403,15 +403,15 @@ func (dc *controller) annotateNodesBackingMachineSets(ctx context.Context, Machi } for _, machine := range filteredMachines { - if machine.Status.Node != "" { + if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { err = AddOrUpdateAnnotationOnNode( ctx, dc.targetCoreClient, - machine.Status.Node, + machine.Labels[v1alpha1.MachineNodeLabelKey], annotations, ) if err != nil { - klog.Warningf("Adding annotation failed for node: %s, %s", machine.Status.Node, err) + klog.Warningf("Adding annotation failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) } } } @@ -454,15 +454,15 @@ func (dc *controller) removeAutoscalerAnnotationsIfRequired(ctx context.Context, } for _, machine := range filteredMachines { - if machine.Status.Node != "" { + if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { nodeAnnotations, err := GetAnnotationsFromNode( ctx, dc.targetCoreClient, - machine.Status.Node, + machine.Labels[v1alpha1.MachineNodeLabelKey], ) if err != nil { - klog.Warningf("Get annotations failed for node: %s, %s", machine.Status.Node, err) + klog.Warningf("Get annotations failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) return err } @@ -472,14 +472,14 @@ func (dc *controller) removeAutoscalerAnnotationsIfRequired(ctx context.Context, err = RemoveAnnotationsOffNode( ctx, dc.targetCoreClient, - machine.Status.Node, + machine.Labels[v1alpha1.MachineNodeLabelKey], annotations, ) if err != nil { - klog.Warningf("Removing annotation failed for node: %s, %s", machine.Status.Node, err) + klog.Warningf("Removing annotation failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) return err } - klog.V(4).Infof("De-annotated the node %q backed by MachineSet %q with %s", machine.Status.Node, machineSet.Name, annotations) + klog.V(4).Infof("De-annotated the node %q backed by MachineSet %q with %s", machine.Labels[v1alpha1.MachineNodeLabelKey], machineSet.Name, annotations) } } } diff --git a/pkg/controller/deployment_rolling_test.go b/pkg/controller/deployment_rolling_test.go index a3b87244c..04d18c430 100644 --- a/pkg/controller/deployment_rolling_test.go +++ b/pkg/controller/deployment_rolling_test.go @@ -128,11 +128,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -293,11 +291,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -347,11 +343,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -400,11 +394,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -453,11 +445,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 0, @@ -597,11 +587,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -653,11 +641,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -709,11 +695,9 @@ var _ = Describe("deployment_rolling", func() { machines: newMachinesFromMachineSet( 1, machineSets[0], - &machinev1.MachineStatus{ - Node: "node", - }, - nil, + &machinev1.MachineStatus{}, nil, + map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, ), nodes: newNodes( 1, diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index 5a21de284..72c04eaa1 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -1514,8 +1514,8 @@ var _ = Describe("machineDeployment", func() { Name: "Machine-test", Namespace: testNamespace, Labels: map[string]string{ - "test-label": "test-label", - "node": "Node1-test", + "test-label": "test-label", + machinev1.MachineNodeLabelKey: "Node1-test", }, UID: "1234567", OwnerReferences: []metav1.OwnerReference{ @@ -1542,7 +1542,6 @@ var _ = Describe("machineDeployment", func() { LastOperation: machinev1.LastOperation{ LastUpdateTime: metav1.Now(), }, - Node: "Node1-test", }, } testNode = &corev1.Node{ diff --git a/pkg/controller/machine.go b/pkg/controller/machine.go index 9b1977dbb..98e5ff6b8 100644 --- a/pkg/controller/machine.go +++ b/pkg/controller/machine.go @@ -197,7 +197,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp } // Sync nodeTemplate between machine and node-objects. - node, _ := c.nodeLister.Get(machine.Status.Node) + node, _ := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) if node != nil { err = c.syncMachineNodeTemplates(ctx, machine) if err != nil { @@ -205,7 +205,6 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return err } } - if machine.DeletionTimestamp != nil { // Processing of delete event if err := c.machineDelete(ctx, machine, driver); err != nil { @@ -298,7 +297,7 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err var ( list = []string{nodeName} selector = labels.NewSelector() - req, _ = labels.NewRequirement("node", selection.Equals, list) + req, _ = labels.NewRequirement(v1alpha1.MachineNodeLabelKey, selection.Equals, list) ) selector = selector.Add(*req) @@ -314,7 +313,10 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err } func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.Machine) (*v1alpha1.Machine, error) { - nodeName := machine.Status.Node + nodeName := "" + if machine.Labels != nil { + nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] + } if nodeName == "" { // Check if any existing node-object can be adopted. @@ -333,10 +335,13 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M klog.V(2).Infof("Adopting the node object %s for machine %s", node.Name, machine.Name) nodeName = node.Name clone := machine.DeepCopy() - clone.Status.Node = nodeName - clone, err = c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{}) + if clone.Labels == nil { + clone.Labels = make(map[string]string) + } + clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName + machine, err = c.controlMachineClient.Machines(clone.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) if err != nil { - klog.Errorf("Could not update status of the machine-object %s due to error %v", machine.Name, err) + klog.Errorf("Could not update the machine-object %s due to error %v", machine.Name, err) return machine, err } break @@ -345,7 +350,7 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M // Couldnt adopt any node-object. if nodeName == "" { // There are no objects mapped to this machine object - // Hence node status need not be propogated to machine object + // Hence return return machine, nil } } @@ -393,25 +398,8 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M } machine, err = c.updateMachineConditions(ctx, machine, node.Status.Conditions) - if err != nil { - return machine, err - } - - clone := machine.DeepCopy() - if clone.Labels == nil { - clone.Labels = make(map[string]string) - } - if n := clone.Labels["node"]; n == "" { - clone.Labels["node"] = machine.Status.Node - machine, err = c.controlMachineClient.Machines(clone.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) - if err != nil { - klog.Warningf("Machine update failed. Retrying, error: %s", err) - return machine, err - } - } - - return machine, nil + return machine, err } /* @@ -542,7 +530,7 @@ func (c *controller) machineCreate(ctx context.Context, machine *v1alpha1.Machin if clone.Labels == nil { clone.Labels = make(map[string]string) } - clone.Labels["node"] = nodeName + clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName if clone.Annotations == nil { clone.Annotations = make(map[string]string) @@ -558,7 +546,6 @@ func (c *controller) machineCreate(ctx context.Context, machine *v1alpha1.Machin } clone = machine.DeepCopy() - clone.Status.Node = nodeName clone.Status.LastOperation = lastOperation clone.Status.CurrentStatus = currentStatus _, err = c.controlMachineClient.Machines(clone.Namespace).UpdateStatus(ctx, clone, metav1.UpdateOptions{}) @@ -613,7 +600,7 @@ func (c *controller) machineUpdate(ctx context.Context, machine *v1alpha1.Machin func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machine, driver driver.Driver) error { var err error - nodeName := machine.Status.Node + nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] if finalizers := sets.NewString(machine.Finalizers...); finalizers.Has(DeleteFinalizerName) { klog.V(2).Infof("Deleting Machine %q", machine.Name) diff --git a/pkg/controller/machine_test.go b/pkg/controller/machine_test.go index a3761519b..bd7ce4c7e 100644 --- a/pkg/controller/machine_test.go +++ b/pkg/controller/machine_test.go @@ -706,7 +706,8 @@ var _ = Describe("machine", func() { actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) Expect(err).To(BeNil()) Expect(actual.Spec).To(Equal(data.expect.machine.Spec)) - Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) + Expect(actual.Labels).ToNot(BeNil()) + Expect(actual.Labels[v1alpha1.MachineNodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey])) //TODO Conditions }, Entry("OpenStackSimple", &data{ @@ -802,9 +803,8 @@ var _ = Describe("machine", func() { ProviderID: "fakeID", }, }, &machinev1.MachineStatus{ - Node: "fakeNode", //TODO conditions - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}), err: false, }, }), @@ -863,9 +863,8 @@ var _ = Describe("machine", func() { ProviderID: "fakeID", }, }, &machinev1.MachineStatus{ - Node: "fakeNode", //TODO conditions - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}), err: false, }, }), @@ -972,9 +971,8 @@ var _ = Describe("machine", func() { ProviderID: "providerid", }, }, &machinev1.MachineStatus{ - Node: "", //TODO conditions - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: ""}), err: false, }, }), @@ -1097,7 +1095,7 @@ var _ = Describe("machine", func() { } if data.action.nodeRecentlyNotReady != nil { - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Status.Node, metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(Not(HaveOccurred())) clone := node.DeepCopy() newNodeCondition := corev1.NodeCondition{ @@ -1128,7 +1126,7 @@ var _ = Describe("machine", func() { Expect(err).ToNot(HaveOccurred()) } - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Status.Node, metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) machine, machineErr := controller.controlMachineClient.Machines(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) if data.expect.machineDeleted { @@ -1376,7 +1374,6 @@ var _ = Describe("machine", func() { fakeNodeName: "fakeNode-0", fakeError: nil, fakeMachineStatus: &machinev1.MachineStatus{ - Node: "fakeNode-0", LastOperation: machinev1.LastOperation{ Description: "Deleting machine from cloud provider", State: v1alpha1.MachineStateProcessing, @@ -1428,7 +1425,6 @@ var _ = Describe("machine", func() { fakeNodeName: "fakeNode-0", fakeError: nil, fakeMachineStatus: &machinev1.MachineStatus{ - Node: "fakeNode-0", LastOperation: machinev1.LastOperation{ Description: "Drain failed - for random reason", State: v1alpha1.MachineStateFailed, @@ -1484,7 +1480,6 @@ var _ = Describe("machine", func() { fakeNodeName: "fakeNode-0", fakeError: nil, fakeMachineStatus: &machinev1.MachineStatus{ - Node: "fakeNode-0", LastOperation: machinev1.LastOperation{ Description: "Drain failed - for random reason", State: v1alpha1.MachineStateFailed, @@ -1900,7 +1895,6 @@ var _ = Describe("machine", func() { Expect(err).To(BeNil()) Expect(actual.Name).To(Equal(data.expect.machine.Name)) - Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) Expect(actual.Status.CurrentStatus.TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.TimeoutActive)) Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) @@ -1908,8 +1902,8 @@ var _ = Describe("machine", func() { Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) if data.expect.machine.Labels != nil { - if _, ok := data.expect.machine.Labels["node"]; ok { - Expect(actual.Labels["node"]).To(Equal(data.expect.machine.Labels["node"])) + if _, ok := data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey]; ok { + Expect(actual.Labels[v1alpha1.MachineNodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey])) } } @@ -1939,9 +1933,7 @@ var _ = Describe("machine", func() { setup: setup{ machines: newMachines(1, &machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - Node: "dummy-node", - }, nil, nil, nil), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), }, action: action{ machine: machineName, @@ -1949,9 +1941,7 @@ var _ = Describe("machine", func() { expect: expect{ machine: newMachine(&machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - Node: "dummy-node", - }, nil, nil, nil), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), }, }), Entry("Machine is running but node object is lost", &data{ @@ -1962,7 +1952,6 @@ var _ = Describe("machine", func() { ObjectMeta: *newObjectMeta(objMeta, 0), }, &machinev1.MachineStatus{ - Node: "dummy-node", CurrentStatus: machinev1.CurrentStatus{ Phase: machinev1.MachineRunning, TimeoutActive: false, @@ -1982,7 +1971,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), }, action: action{ machine: machineName, @@ -1991,7 +1980,6 @@ var _ = Describe("machine", func() { machine: newMachine(&machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), }, &machinev1.MachineStatus{ - Node: "dummy-node", CurrentStatus: machinev1.CurrentStatus{ Phase: machinev1.MachineUnknown, TimeoutActive: true, @@ -2014,7 +2002,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), }, }), Entry("Machine and node both are present and kubelet ready status is updated", &data{ @@ -2022,7 +2010,6 @@ var _ = Describe("machine", func() { machines: newMachines(1, &machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), }, &machinev1.MachineStatus{ - Node: "machine", CurrentStatus: machinev1.CurrentStatus{ Phase: machinev1.MachinePending, TimeoutActive: true, @@ -2042,7 +2029,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), nodes: []*corev1.Node{ { TypeMeta: metav1.TypeMeta{ @@ -2050,7 +2037,7 @@ var _ = Describe("machine", func() { APIVersion: "v1", }, ObjectMeta: metav1.ObjectMeta{ - Name: "machine-0", + Name: "node-0", }, Status: corev1.NodeStatus{ Conditions: []corev1.NodeCondition{ @@ -2072,7 +2059,6 @@ var _ = Describe("machine", func() { machine: newMachine(&machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), }, &machinev1.MachineStatus{ - Node: "machine", CurrentStatus: machinev1.CurrentStatus{ Phase: machinev1.MachineRunning, TimeoutActive: false, @@ -2092,42 +2078,10 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, nil), + }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), }, }), - Entry("Machine object does not have node-label and node exists", &data{ - setup: setup{ - machines: newMachines(1, &machinev1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{ - Node: "node", - }, nil, nil, nil), - nodes: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-0", - }, - }, - }, - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&machinev1.MachineTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Name: "machine-0", - }, - }, &machinev1.MachineStatus{ - Node: "node", - }, nil, nil, - map[string]string{ - "node": "node-0", - }, - ), - }, - }), - Entry("Machine object does not have status.node set and node exists then it should adopt node using providerID", &data{ + Entry("Machine object does not have node label set and node exists then it should adopt node using providerID", &data{ setup: setup{ machines: newMachines(1, &machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), @@ -2154,16 +2108,10 @@ var _ = Describe("machine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "machine-0", }, - }, &machinev1.MachineStatus{ - Node: "node", - }, nil, nil, - map[string]string{ - "node": "node-0", - }, - ), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), }, }), - Entry("Machine object does not have status.node set and node exists (without providerID) then it should not adopt node using providerID", &data{ + Entry("Machine object does not have node label set and node exists (without providerID) then it should not adopt node using providerID", &data{ setup: setup{ machines: newMachines(1, &machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), diff --git a/pkg/controller/machine_util.go b/pkg/controller/machine_util.go index 0907eca0e..e1eb8042b 100644 --- a/pkg/controller/machine_util.go +++ b/pkg/controller/machine_util.go @@ -303,7 +303,7 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al currentlyAppliedALTJSONByte []byte ) - node, err := c.nodeLister.Get(machine.Status.Node) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) if err != nil || node == nil { klog.Errorf("Error: Could not get the node-object or node-object is missing - err: %q", err) // Dont return error so that other steps can be executed. @@ -516,8 +516,7 @@ func (c *controller) UpdateNodeTerminationCondition(ctx context.Context, machine return nil } - nodeName := machine.Status.Node - + nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] terminationCondition := v1.NodeCondition{ Type: NodeTerminationCondition, Status: v1.ConditionTrue, diff --git a/pkg/controller/machine_util_test.go b/pkg/controller/machine_util_test.go index 5b648770e..b141a2e8b 100644 --- a/pkg/controller/machine_util_test.go +++ b/pkg/controller/machine_util_test.go @@ -126,10 +126,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -223,10 +221,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -304,10 +300,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{}, @@ -358,10 +352,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -1915,10 +1907,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineFailed}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -1961,10 +1952,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineRunning}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2007,10 +1997,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2053,10 +2042,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2103,10 +2091,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineFailed}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2148,10 +2135,9 @@ var _ = Describe("machine_util", func() { machine: newMachine( &machinev1.MachineTemplateSpec{}, &machinev1.MachineStatus{ - Node: "test-node", CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, nil), + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{}, diff --git a/pkg/controller/secret.go b/pkg/controller/secret.go index 91924cd88..d4e94aace 100644 --- a/pkg/controller/secret.go +++ b/pkg/controller/secret.go @@ -32,7 +32,7 @@ import ( "k8s.io/klog/v2" ) -// reconcileClusterSecretKey reconciles an secret due to controller resync +// reconcileClusterSecretKey reconciles a secret due to controller resync // or an event on the secret func (c *controller) reconcileClusterSecretKey(key string) error { ctx := context.Background() diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go index 1957b5349..58677cffa 100644 --- a/pkg/openapi/openapi_generated.go +++ b/pkg/openapi/openapi_generated.go @@ -3285,13 +3285,6 @@ func schema_pkg_apis_machine_v1alpha1_MachineStatus(ref common.ReferenceCallback Description: "MachineStatus holds the most recently observed status of Machine.", Type: []string{"object"}, Properties: map[string]spec.Schema{ - "node": { - SchemaProps: spec.SchemaProps{ - Description: "Node string", - Type: []string{"string"}, - Format: "", - }, - }, "conditions": { SchemaProps: spec.SchemaProps{ Description: "Conditions of this machine, same as node", diff --git a/pkg/util/provider/machinecontroller/controller_suite_test.go b/pkg/util/provider/machinecontroller/controller_suite_test.go index ac28e58e1..7eb2e5b03 100644 --- a/pkg/util/provider/machinecontroller/controller_suite_test.go +++ b/pkg/util/provider/machinecontroller/controller_suite_test.go @@ -455,13 +455,7 @@ func newMachineStatus(statusTemplate *v1alpha1.MachineStatus, index int) *v1alph return &v1alpha1.MachineStatus{} } - r := statusTemplate.DeepCopy() - if r.Node == "" { - return r - } - - r.Node = fmt.Sprintf("%s-%d", r.Node, index) - return r + return statusTemplate.DeepCopy() } func newSecretReference(meta *metav1.ObjectMeta, index int) *corev1.SecretReference { diff --git a/pkg/util/provider/machinecontroller/machine.go b/pkg/util/provider/machinecontroller/machine.go index ed6e29ec2..999588133 100644 --- a/pkg/util/provider/machinecontroller/machine.go +++ b/pkg/util/provider/machinecontroller/machine.go @@ -161,7 +161,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } - if machine.Status.Node != "" { + if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { // If reference to node object exists execute the below retry, err := c.reconcileMachineHealth(ctx, machine) if err != nil { @@ -173,7 +173,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } } - if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" || machine.Status.Node == "" { + if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" || machine.Labels[v1alpha1.MachineNodeLabelKey] == "" { return c.triggerCreationFlow( ctx, &driver.CreateMachineRequest{ @@ -301,23 +301,6 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err return machines[0], nil } -/* - Move to update method? - clone := machine.DeepCopy() - if clone.Labels == nil { - clone.Labels = make(map[string]string) - } - - if _, ok := clone.Labels["node"]; !ok { - clone.Labels["node"] = machine.Status.Node - machine, err = c.controlMachineClient.Machines(clone.Namespace).Update(clone) - if err != nil { - klog.Warningf("Machine update failed. Retrying, error: %s", err) - return machine, err - } - } -*/ - /* SECTION Machine operations - Create, Update, Delete @@ -366,7 +349,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque // Either VM is not found // or GetMachineStatus() call is not implemented // In this case, invoke a CreateMachine() call - if _, present := machine.Labels["node"]; !present { + if _, present := machine.Labels[v1alpha1.MachineNodeLabelKey]; !present { // If node label is not present klog.V(2).Infof("Creating a VM for machine %q, please wait!", machine.Name) klog.V(2).Infof("The machine creation is triggered with timeout of %s", c.getEffectiveCreationTimeout(createMachineRequest.Machine).Duration) @@ -434,7 +417,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque } } else { // if node label present that means there must be a backing VM ,without need of GetMachineStatus() call - nodeName = machine.Labels["node"] + nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] } case codes.Unknown, codes.DeadlineExceeded, codes.Aborted, codes.Unavailable: @@ -478,14 +461,14 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque return machineutils.MediumRetry, err } } else { - if machine.Labels["node"] == "" || machine.Spec.ProviderID == "" { + if machine.Labels[v1alpha1.MachineNodeLabelKey] == "" || machine.Spec.ProviderID == "" { klog.V(2).Infof("Found VM with required machine name. Adopting existing machine: %q with ProviderID: %s", machineName, getMachineStatusResponse.ProviderID) } nodeName = getMachineStatusResponse.NodeName providerID = getMachineStatusResponse.ProviderID } - _, machineNodeLabelPresent := createMachineRequest.Machine.Labels["node"] + _, machineNodeLabelPresent := createMachineRequest.Machine.Labels[v1alpha1.MachineNodeLabelKey] _, machinePriorityAnnotationPresent := createMachineRequest.Machine.Annotations[machineutils.MachinePriority] if !machineNodeLabelPresent || !machinePriorityAnnotationPresent || machine.Spec.ProviderID == "" { @@ -493,7 +476,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque if clone.Labels == nil { clone.Labels = make(map[string]string) } - clone.Labels["node"] = nodeName + clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName if clone.Annotations == nil { clone.Annotations = make(map[string]string) } @@ -514,10 +497,8 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque return machineutils.ShortRetry, err } - if machine.Status.Node != nodeName || machine.Status.CurrentStatus.Phase == "" { + if machine.Status.CurrentStatus.Phase == "" { clone := machine.DeepCopy() - - clone.Status.Node = nodeName clone.Status.LastOperation = v1alpha1.LastOperation{ Description: "Creating machine on cloud provider", State: v1alpha1.MachineStateProcessing, diff --git a/pkg/util/provider/machinecontroller/machine_safety_test.go b/pkg/util/provider/machinecontroller/machine_safety_test.go index eccad9197..97782a982 100644 --- a/pkg/util/provider/machinecontroller/machine_safety_test.go +++ b/pkg/util/provider/machinecontroller/machine_safety_test.go @@ -447,7 +447,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_1", Namespace: testNamespace, Labels: map[string]string{ - "node": "test-node-1", + v1alpha1.MachineNodeLabelKey: "test-node-1", }, }, Status: v1alpha1.MachineStatus{ @@ -464,7 +464,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_2", Namespace: testNamespace, Labels: map[string]string{ - "node": "test-node-2", + v1alpha1.MachineNodeLabelKey: "test-node-2", }, }, Status: v1alpha1.MachineStatus{ @@ -481,7 +481,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_3", Namespace: testNamespace, Labels: map[string]string{ - "node": "test-node-2", + v1alpha1.MachineNodeLabelKey: "test-node-2", }, }, Status: v1alpha1.MachineStatus{ diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go index 782fbc5a1..1c45d56f9 100644 --- a/pkg/util/provider/machinecontroller/machine_test.go +++ b/pkg/util/provider/machinecontroller/machine_test.go @@ -122,83 +122,7 @@ var _ = Describe("machine", func() { Entry("with NodeReady is Unknown", corev1.NodeReady, corev1.ConditionUnknown, false), ) }) - - /* - Describe("##updateMachineConditions", func() { - Describe("Update conditions of a non-existing machine", func() { - It("should return error", func() { - stop := make(chan struct{}) - defer close(stop) - - objects := []runtime.Object{} - c, trackers := createController(stop, testNamespace, objects, nil, nil) - defer trackers.Stop() - - testMachine := &v1alpha1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testmachine", - Namespace: testNamespace, - }, - Status: v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineTerminating, - }, - }, - } - conditions := []corev1.NodeCondition{} - var _, err = c.updateMachineConditions(testMachine, conditions) - Expect(err).Should(Not(BeNil())) - }) - }) - DescribeTable("Update conditions of an existing machine", - func(phase v1alpha1.MachinePhase, conditions []corev1.NodeCondition, expectedPhase v1alpha1.MachinePhase) { - stop := make(chan struct{}) - defer close(stop) - - testMachine := &v1alpha1.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: "testmachine", - Namespace: testNamespace, - }, - Status: v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: phase, - }, - }, - } - objects := []runtime.Object{} - objects = append(objects, testMachine) - - c, trackers := createController(stop, testNamespace, objects, nil, nil) - defer trackers.Stop() - - var updatedMachine, err = c.updateMachineConditions(testMachine, conditions) - Expect(updatedMachine.Status.Conditions).Should(BeEquivalentTo(conditions)) - Expect(updatedMachine.Status.CurrentStatus.Phase).Should(BeIdenticalTo(expectedPhase)) - Expect(err).Should(BeNil()) - }, - Entry("healthy status but machine terminating", v1alpha1.MachineTerminating, []corev1.NodeCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionTrue, - }, - }, v1alpha1.MachineTerminating), - Entry("unhealthy status but machine running", v1alpha1.MachineRunning, []corev1.NodeCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionFalse, - }, - }, v1alpha1.MachineUnknown), - Entry("healthy status but machine not running", v1alpha1.MachineAvailable, []corev1.NodeCondition{ - { - Type: corev1.NodeReady, - Status: corev1.ConditionTrue, - }, - }, v1alpha1.MachineRunning), - ) - }) - */ - + Describe("#ValidateMachine", func() { type data struct { action machineapi.Machine @@ -470,10 +394,14 @@ var _ = Describe("machine", func() { actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) Expect(err).To(BeNil()) Expect(actual.Spec.ProviderID).To(Equal(data.expect.machine.Spec.ProviderID)) - Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) Expect(actual.Finalizers).To(Equal(data.expect.machine.Finalizers)) Expect(retry).To(Equal(data.expect.retry)) Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) + if data.expect.machine.Labels == nil { + Expect(actual.Labels).To(BeNil()) + } else { + Expect(actual.Labels).To(Equal(data.expect.machine.Labels)) + } }, Entry("Machine creation succeeds with object UPDATE", &data{ @@ -519,7 +447,7 @@ var _ = Describe("machine", func() { }, ProviderID: "fakeID", }, - }, nil, nil, nil, nil, true, metav1.Now()), + }, nil, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}, true, metav1.Now()), err: fmt.Errorf("Machine creation in process. Machine UPDATE successful"), retry: machineutils.ShortRetry, }, @@ -556,7 +484,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -584,7 +512,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachinePending, }, @@ -594,7 +521,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -630,7 +557,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.Now(), @@ -647,7 +573,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -675,7 +601,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachinePending, @@ -693,7 +618,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1021,11 +946,11 @@ var _ = Describe("machine", func() { Expect(machine.Finalizers).To(Equal(data.expect.machine.Finalizers)) if data.expect.nodeDeleted { - _, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Status.Node, metav1.GetOptions{}) + _, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(HaveOccurred()) } if data.expect.nodeTerminationConditionIsSet { - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Status.Node, metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(Not(HaveOccurred())) Expect(len(node.Status.Conditions)).To(Equal(1)) Expect(node.Status.Conditions[0].Type).To(Equal(machineutils.NodeTerminationCondition)) @@ -1059,7 +984,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now(), @@ -1076,7 +1000,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -1106,7 +1030,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now(), @@ -1123,7 +1046,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -1156,7 +1079,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now(), @@ -1173,7 +1095,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1203,7 +1125,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1220,7 +1141,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1253,7 +1174,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1270,7 +1190,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1300,7 +1220,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1317,7 +1236,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1350,7 +1269,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1367,7 +1285,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeNode-0", + v1alpha1.MachineNodeLabelKey: "fakeNode-0", }, true, metav1.Now(), @@ -1405,7 +1323,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1422,7 +1339,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1455,7 +1372,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1479,7 +1395,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "", + v1alpha1.MachineNodeLabelKey: "", }, true, metav1.Now(), @@ -1509,7 +1425,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1526,7 +1441,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "", + v1alpha1.MachineNodeLabelKey: "", }, true, metav1.Now(), @@ -1559,7 +1474,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1583,7 +1497,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1613,7 +1527,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1630,7 +1543,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1663,7 +1576,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1687,7 +1599,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1717,7 +1629,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1734,7 +1645,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1767,7 +1678,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1796,7 +1706,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1826,7 +1736,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1843,7 +1752,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1876,7 +1785,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1900,7 +1808,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1930,7 +1838,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -1947,7 +1854,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1980,7 +1887,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2004,7 +1910,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2034,7 +1940,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2051,7 +1956,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2084,7 +1989,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2101,8 +2005,8 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", - "force-deletion": "True", + v1alpha1.MachineNodeLabelKey: "fakeID-0", + "force-deletion": "True", }, true, metav1.Now(), @@ -2144,7 +2048,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2161,7 +2064,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2194,7 +2097,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.NewTime(time.Now().Add(-3 * time.Minute)), @@ -2211,7 +2113,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.NewTime(time.Now().Add(-3*time.Minute)), @@ -2253,7 +2155,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2270,7 +2171,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2303,7 +2204,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.NewTime(time.Now().Add(-2 * time.Hour)), @@ -2320,7 +2220,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.NewTime(time.Now().Add(-3*time.Hour)), @@ -2362,7 +2262,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2379,7 +2278,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2412,7 +2311,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2429,7 +2327,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeNode-0", + v1alpha1.MachineNodeLabelKey: "fakeNode-0", }, true, metav1.Now(), @@ -2471,7 +2369,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2488,7 +2385,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2521,7 +2418,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2538,7 +2434,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2568,7 +2464,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2585,7 +2480,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2618,7 +2513,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeID", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2635,7 +2529,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2673,7 +2567,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2690,7 +2583,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2723,7 +2616,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2740,7 +2632,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2769,7 +2661,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2786,7 +2677,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -2819,7 +2710,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2836,7 +2726,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2866,7 +2756,6 @@ var _ = Describe("machine", func() { }, }, &v1alpha1.MachineStatus{ - Node: "fakeNode", CurrentStatus: v1alpha1.CurrentStatus{ Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.Now(), @@ -2883,7 +2772,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - "node": "fakeID-0", + v1alpha1.MachineNodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2892,495 +2781,5 @@ var _ = Describe("machine", func() { }), ) }) - /* - Describe("#checkMachineTimeout", func() { - type setup struct { - machines []*v1alpha1.Machine - } - type action struct { - machine string - } - type expect struct { - machine *v1alpha1.Machine - err bool - } - type data struct { - setup setup - action action - expect expect - } - objMeta := &metav1.ObjectMeta{ - GenerateName: "machine", - Namespace: "test", - } - - machineName := "machine-0" - timeOutOccurred := -21 * time.Minute - timeOutNotOccurred := -5 * time.Minute - creationTimeOut := 20 * time.Minute - healthTimeOut := 10 * time.Minute - - DescribeTable("##Machine Timeout Scenarios", - func(data *data) { - stop := make(chan struct{}) - defer close(stop) - - machineObjects := []runtime.Object{} - for _, o := range data.setup.machines { - machineObjects = append(machineObjects, o) - } - - coreObjects := []runtime.Object{} - - controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects) - defer trackers.Stop() - waitForCacheSync(stop, controller) - - action := data.action - machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) - //Expect(err).ToNot(HaveOccurred()) - - controller.checkMachineTimeout(machine) - - actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) - Expect(err).To(BeNil()) - Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) - Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) - Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) - Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) - Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) - }, - Entry("Machine is still running", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineRunning, - //TimeoutActive: false, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: v1alpha1.MachineStateSuccessful, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), - }, - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineRunning, - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: v1alpha1.MachineStateSuccessful, - Type: v1alpha1.MachineOperationCreate, - }, - }, nil, nil, nil), - }, - }), - Entry("Machine creation has still not timed out", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineUnknown, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), - }, - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineUnknown, - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationCreate, - }, - }, nil, nil, nil), - }, - }), - Entry("Machine creation has timed out", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachinePending, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), - }, - LastOperation: v1alpha1.LastOperation{ - Description: "Creating machine on cloud provider", - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), - }, - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineFailed, - - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf( - "Machine %s failed to join the cluster in %s minutes.", - machineName, - creationTimeOut, - ), - State: v1alpha1.MachineStateFailed, - Type: v1alpha1.MachineOperationCreate, - }, - }, nil, nil, nil), - }, - }), - Entry("Machine health has timed out", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineUnknown, - - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationHealthCheck, - LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), - }, - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineFailed, - - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf( - "Machine %s is not healthy since %s minutes. Changing status to failed. Node Conditions: %+v", - machineName, - healthTimeOut, - []corev1.NodeCondition{}, - ), - State: v1alpha1.MachineStateFailed, - Type: v1alpha1.MachineOperationHealthCheck, - }, - }, nil, nil, nil), - }, - }), - ) - }) - - Describe("#updateMachineState", func() { - type setup struct { - machines []*v1alpha1.Machine - nodes []*corev1.Node - } - type action struct { - machine string - } - type expect struct { - machine *v1alpha1.Machine - err bool - } - type data struct { - setup setup - action action - expect expect - } - objMeta := &metav1.ObjectMeta{ - GenerateName: "machine", - // using default namespace for non-namespaced objects - // as our current fake client is with the assumption - // that all objects are namespaced - Namespace: "", - } - - machineName := "machine-0" - - DescribeTable("##Different machine state update scenrios", - func(data *data) { - stop := make(chan struct{}) - defer close(stop) - - machineObjects := []runtime.Object{} - for _, o := range data.setup.machines { - machineObjects = append(machineObjects, o) - } - - coreObjects := []runtime.Object{} - for _, o := range data.setup.nodes { - coreObjects = append(coreObjects, o) - } - - controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects) - defer trackers.Stop() - waitForCacheSync(stop, controller) - - action := data.action - machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) - Expect(err).ToNot(HaveOccurred()) - - controller.updateMachineState(machine) - - actual, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) - Expect(err).To(BeNil()) - Expect(actual.Name).To(Equal(data.expect.machine.Name)) - Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) - Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) - Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) - Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) - Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) - Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) - - if data.expect.machine.Labels != nil { - if _, ok := data.expect.machine.Labels["node"]; ok { - Expect(actual.Labels["node"]).To(Equal(data.expect.machine.Labels["node"])) - } - } - - for i := range actual.Status.Conditions { - Expect(actual.Status.Conditions[i].Type).To(Equal(data.expect.machine.Status.Conditions[i].Type)) - Expect(actual.Status.Conditions[i].Status).To(Equal(data.expect.machine.Status.Conditions[i].Status)) - Expect(actual.Status.Conditions[i].Reason).To(Equal(data.expect.machine.Status.Conditions[i].Reason)) - Expect(actual.Status.Conditions[i].Message).To(Equal(data.expect.machine.Status.Conditions[i].Message)) - } - }, - Entry("Machine does not have a node backing", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{}, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{}, nil, nil, nil), - }, - }), - Entry("Node object backing machine not found and machine conditions are empty", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "dummy-node", - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "dummy-node", - }, nil, nil, nil), - }, - }), - Entry("Machine is running but node object is lost", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "dummy-node", - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineRunning, - //TimeoutActive: false, - LastUpdateTime: metav1.Now(), - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), - State: v1alpha1.MachineStateSuccessful, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.Now(), - }, - Conditions: []corev1.NodeCondition{ - { - Message: "kubelet is posting ready status", - Reason: "KubeletReady", - Status: "True", - Type: "Ready", - }, - }, - }, nil, nil, nil), - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "dummy-node", - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineUnknown, - - LastUpdateTime: metav1.Now(), - }, - LastOperation: v1alpha1.LastOperation{ - Description: fmt.Sprintf( - "Node object went missing. Machine %s is unhealthy - changing MachineState to Unknown", - machineName, - ), - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationHealthCheck, - LastUpdateTime: metav1.Now(), - }, - Conditions: []corev1.NodeCondition{ - { - Message: "kubelet is posting ready status", - Reason: "KubeletReady", - Status: "True", - Type: "Ready", - }, - }, - }, nil, nil, nil), - }, - }), - Entry("Machine and node both are present and kubelet ready status is updated", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "machine", - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachinePending, - - LastUpdateTime: metav1.Now(), - }, - LastOperation: v1alpha1.LastOperation{ - Description: "Creating machine on cloud provider", - State: v1alpha1.MachineStateProcessing, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.Now(), - }, - Conditions: []corev1.NodeCondition{ - { - Message: "kubelet is not ready", - Reason: "KubeletReady", - Status: "False", - Type: "Ready", - }, - }, - }, nil, nil, nil), - nodes: []*corev1.Node{ - { - ObjectMeta: *newObjectMeta(objMeta, 0), - Status: corev1.NodeStatus{ - Conditions: []corev1.NodeCondition{ - { - Message: "kubelet is posting ready status", - Reason: "KubeletReady", - Status: "True", - Type: "Ready", - }, - }, - }, - }, - }, - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "machine", - CurrentStatus: v1alpha1.CurrentStatus{ - Phase: v1alpha1.MachineRunning, - //TimeoutActive: false, - LastUpdateTime: metav1.Now(), - }, - LastOperation: v1alpha1.LastOperation{ - Description: "Machine machine-0 successfully joined the cluster", - State: v1alpha1.MachineStateSuccessful, - Type: v1alpha1.MachineOperationCreate, - LastUpdateTime: metav1.Now(), - }, - Conditions: []corev1.NodeCondition{ - { - Message: "kubelet is posting ready status", - Reason: "KubeletReady", - Status: "True", - Type: "Ready", - }, - }, - }, nil, nil, nil), - }, - }), - Entry("Machine object does not have node-label and node exists", &data{ - setup: setup{ - machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ - ObjectMeta: *newObjectMeta(objMeta, 0), - }, &v1alpha1.MachineStatus{ - Node: "node", - }, nil, nil, nil), - nodes: []*corev1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node-0", - }, - }, - }, - }, - action: action{ - machine: machineName, - }, - expect: expect{ - machine: newMachine(&v1alpha1.MachineTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Name: "machine-0", - }, - }, &v1alpha1.MachineStatus{ - Node: "node", - }, nil, nil, - map[string]string{ - "node": "node-0", - }, - ), - }, - }), - ) - }) - */ }) diff --git a/pkg/util/provider/machinecontroller/machine_util.go b/pkg/util/provider/machinecontroller/machine_util.go index 92fa47498..10a312c0c 100644 --- a/pkg/util/provider/machinecontroller/machine_util.go +++ b/pkg/util/provider/machinecontroller/machine_util.go @@ -256,7 +256,7 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al lastAppliedALT v1alpha1.NodeTemplateSpec ) - node, err := c.nodeLister.Get(machine.Status.Node) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) if err != nil && apierrors.IsNotFound(err) { // Dont return error so that other steps can be executed. return machineutils.LongRetry, nil @@ -589,7 +589,7 @@ func (c *controller) reconcileMachineHealth(ctx context.Context, machine *v1alph lastOperationType v1alpha1.MachineOperationType ) - node, err := c.nodeLister.Get(machine.Status.Node) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) if err != nil { if apierrors.IsNotFound(err) { // Node object is not found @@ -987,7 +987,7 @@ func (c *controller) drainNode(ctx context.Context, deleteMachineRequest *driver pvReattachTimeOut = c.safetyOptions.PvReattachTimeout.Duration timeOutDuration = c.getEffectiveDrainTimeout(deleteMachineRequest.Machine).Duration forceDeleteLabelPresent = machine.Labels["force-deletion"] == "True" - nodeName = machine.Labels["node"] + nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] nodeNotReadyDuration = 5 * time.Minute ReadonlyFilesystem v1.NodeConditionType = "ReadonlyFilesystem" ) @@ -1213,7 +1213,7 @@ func (c *controller) deleteNodeObject(ctx context.Context, machine *v1alpha1.Mac state v1alpha1.MachineState ) - nodeName := machine.Labels["node"] + nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] if nodeName != "" { // Delete node object @@ -1318,7 +1318,7 @@ func (c *controller) UpdateNodeTerminationCondition(ctx context.Context, machine return nil } - nodeName := machine.Labels["node"] + nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] terminationCondition := v1.NodeCondition{ Type: machineutils.NodeTerminationCondition, @@ -1515,7 +1515,7 @@ func getProviderID(machine *v1alpha1.Machine) string { } func getNodeName(machine *v1alpha1.Machine) string { - return machine.Status.Node + return machine.Labels[v1alpha1.MachineNodeLabelKey] } func getMachineDeploymentName(machine *v1alpha1.Machine) string { diff --git a/pkg/util/provider/machinecontroller/machine_util_test.go b/pkg/util/provider/machinecontroller/machine_util_test.go index f8e4af29a..b368ffcca 100644 --- a/pkg/util/provider/machinecontroller/machine_util_test.go +++ b/pkg/util/provider/machinecontroller/machine_util_test.go @@ -142,10 +142,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -239,10 +237,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -320,10 +316,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{}, @@ -374,10 +368,8 @@ var _ = Describe("machine_util", func() { }, }, }, - &machinev1.MachineStatus{ - Node: "test-node", - }, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -2179,8 +2171,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node-0", CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-25 * time.Minute))}}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-25 * time.Minute))}}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2195,8 +2187,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.Now()}}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.Now()}}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, false, false, false, false)}), @@ -2214,8 +2206,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2233,8 +2225,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, true, false, false, false)}), @@ -2252,8 +2244,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}, Conditions: nodeConditions(true, false, false, false, false)}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}, Conditions: nodeConditions(true, false, false, false, false)}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2268,9 +2260,9 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2287,9 +2279,9 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2303,8 +2295,8 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node-0", Conditions: nodeConditions(true, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, - nil, nil, nil, true, metav1.Now()), + &machinev1.MachineStatus{Conditions: nodeConditions(true, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, false, false, false, false)}), @@ -2380,14 +2372,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy2}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2404,14 +2396,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy2}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2427,9 +2419,9 @@ var _ = Describe("machine_util", func() { setup: setup{ machines: newMachines(2, &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, nil, true, metav1.Now()), + nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), }, @@ -2445,14 +2437,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2470,14 +2462,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2495,14 +2487,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: "", LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: "", LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2520,14 +2512,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2545,14 +2537,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineCrashLoopBackOff, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineCrashLoopBackOff, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2570,14 +2562,14 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet2Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2595,9 +2587,9 @@ var _ = Describe("machine_util", func() { machines: []*v1alpha1.Machine{ newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, - &machinev1.MachineStatus{Node: "node", Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, + &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), From 345b365d166be1de57b62ddca007c55c1d81827f Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 19 Sep 2022 17:35:29 +0530 Subject: [PATCH 2/3] renaming node label constant --- Makefile | 2 +- pkg/apis/machine/types.go | 5 -- pkg/apis/machine/v1alpha1/machine_types.go | 10 +-- pkg/controller/deployment_rollback.go | 12 +-- pkg/controller/deployment_rollback_test.go | 2 +- pkg/controller/deployment_rolling.go | 24 ++--- pkg/controller/deployment_rolling_test.go | 16 ++-- pkg/controller/deployment_test.go | 4 +- pkg/controller/machine.go | 12 +-- pkg/controller/machine_test.go | 30 +++---- pkg/controller/machine_util.go | 4 +- pkg/controller/machine_util_test.go | 20 ++--- .../provider/machinecontroller/machine.go | 14 +-- .../machinecontroller/machine_safety_test.go | 6 +- .../machinecontroller/machine_test.go | 90 +++++++++---------- .../machinecontroller/machine_util.go | 12 +-- .../machinecontroller/machine_util_test.go | 60 ++++++------- 17 files changed, 159 insertions(+), 164 deletions(-) diff --git a/Makefile b/Makefile index bf579ad8e..b748590a0 100644 --- a/Makefile +++ b/Makefile @@ -149,7 +149,7 @@ test-clean: @rm -f $(COVERPROFILE) generate: controller-gen - $(CONTROLLER_GEN) crd paths=./pkg/apis/machine/v1alpha1... output:crd:dir=kubernetes/crds output:stdout + $(CONTROLLER_GEN) crd paths=./pkg/apis/machine/v1alpha1/... output:crd:dir=kubernetes/crds output:stdout @./hack/generate-code @./hack/api-reference/generate-spec-doc.sh diff --git a/pkg/apis/machine/types.go b/pkg/apis/machine/types.go index 57c1a3c80..46e1d241b 100644 --- a/pkg/apis/machine/types.go +++ b/pkg/apis/machine/types.go @@ -33,11 +33,6 @@ import ( // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object -// This is the valid key for machine node label -const ( - MachineNodeLabelKey string = "node" -) - // Machine TODO type Machine struct { // ObjectMeta for machine object diff --git a/pkg/apis/machine/v1alpha1/machine_types.go b/pkg/apis/machine/v1alpha1/machine_types.go index fbce5eca3..36f0bb99d 100644 --- a/pkg/apis/machine/v1alpha1/machine_types.go +++ b/pkg/apis/machine/v1alpha1/machine_types.go @@ -27,6 +27,11 @@ import ( // IF YOU MODIFY ANY OF THE TYPES HERE COPY THEM TO ../types.go // AND RUN ./hack/generate-code +// NodeLabelKey is the key for node label on machine object +const ( + NodeLabelKey string = "node" +) + // +genclient // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // +kubebuilder:object:root=true @@ -35,11 +40,6 @@ import ( // +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp`,description="CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.\nPopulated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata" // +kubebuilder:printcolumn:name="Node",type=string,JSONPath=`.metadata.labels.node`,description="Node backing the machine object" -// This is the valid key for machine node label -const ( - MachineNodeLabelKey string = "node" -) - // Machine is the representation of a physical or virtual machine. type Machine struct { // ObjectMeta for machine object diff --git a/pkg/controller/deployment_rollback.go b/pkg/controller/deployment_rollback.go index cf477bae3..a55062aa4 100644 --- a/pkg/controller/deployment_rollback.go +++ b/pkg/controller/deployment_rollback.go @@ -172,24 +172,24 @@ func (dc *controller) removeTaintNodesBackingMachineSet(ctx context.Context, mac // Iterate through all machines and remove the PreferNoSchedule taint // to avoid scheduling on older machines for _, machine := range filteredMachines { - if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { - node, err := dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + if machine.Labels[v1alpha1.NodeLabelKey] != "" { + node, err := dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) if err != nil { - klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.NodeLabelKey], err) continue } err = nodeops.RemoveTaintOffNode( ctx, dc.targetCoreClient, - machine.Labels[v1alpha1.MachineNodeLabelKey], + machine.Labels[v1alpha1.NodeLabelKey], node, taint, ) if err != nil { - klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Node taint removal failed for node: %s, Error: %s", machine.Labels[v1alpha1.NodeLabelKey], err) } - node, err = dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + node, err = dc.targetCoreClient.CoreV1().Nodes().Get(ctx, machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) } } diff --git a/pkg/controller/deployment_rollback_test.go b/pkg/controller/deployment_rollback_test.go index a41a4aecf..f9ee49b86 100644 --- a/pkg/controller/deployment_rollback_test.go +++ b/pkg/controller/deployment_rollback_test.go @@ -133,7 +133,7 @@ var _ = Describe("deployment_rollback", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, diff --git a/pkg/controller/deployment_rolling.go b/pkg/controller/deployment_rolling.go index b1918fecc..ff0716cd5 100644 --- a/pkg/controller/deployment_rolling.go +++ b/pkg/controller/deployment_rolling.go @@ -322,15 +322,15 @@ func (dc *controller) taintNodesBackingMachineSets(ctx context.Context, MachineS // Iterate through all machines and place the PreferNoSchedule taint // to avoid scheduling on older machines for _, machine := range filteredMachines { - if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { + if machine.Labels[v1alpha1.NodeLabelKey] != "" { err = nodeops.AddOrUpdateTaintOnNode( ctx, dc.targetCoreClient, - machine.Labels[v1alpha1.MachineNodeLabelKey], + machine.Labels[v1alpha1.NodeLabelKey], taint, ) if err != nil { - klog.Warningf("Node tainting failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Node tainting failed for node: %s, %s", machine.Labels[v1alpha1.NodeLabelKey], err) } } } @@ -403,15 +403,15 @@ func (dc *controller) annotateNodesBackingMachineSets(ctx context.Context, Machi } for _, machine := range filteredMachines { - if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { + if machine.Labels[v1alpha1.NodeLabelKey] != "" { err = AddOrUpdateAnnotationOnNode( ctx, dc.targetCoreClient, - machine.Labels[v1alpha1.MachineNodeLabelKey], + machine.Labels[v1alpha1.NodeLabelKey], annotations, ) if err != nil { - klog.Warningf("Adding annotation failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Adding annotation failed for node: %s, %s", machine.Labels[v1alpha1.NodeLabelKey], err) } } } @@ -454,15 +454,15 @@ func (dc *controller) removeAutoscalerAnnotationsIfRequired(ctx context.Context, } for _, machine := range filteredMachines { - if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { + if machine.Labels[v1alpha1.NodeLabelKey] != "" { nodeAnnotations, err := GetAnnotationsFromNode( ctx, dc.targetCoreClient, - machine.Labels[v1alpha1.MachineNodeLabelKey], + machine.Labels[v1alpha1.NodeLabelKey], ) if err != nil { - klog.Warningf("Get annotations failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Get annotations failed for node: %s, %s", machine.Labels[v1alpha1.NodeLabelKey], err) return err } @@ -472,14 +472,14 @@ func (dc *controller) removeAutoscalerAnnotationsIfRequired(ctx context.Context, err = RemoveAnnotationsOffNode( ctx, dc.targetCoreClient, - machine.Labels[v1alpha1.MachineNodeLabelKey], + machine.Labels[v1alpha1.NodeLabelKey], annotations, ) if err != nil { - klog.Warningf("Removing annotation failed for node: %s, %s", machine.Labels[v1alpha1.MachineNodeLabelKey], err) + klog.Warningf("Removing annotation failed for node: %s, %s", machine.Labels[v1alpha1.NodeLabelKey], err) return err } - klog.V(4).Infof("De-annotated the node %q backed by MachineSet %q with %s", machine.Labels[v1alpha1.MachineNodeLabelKey], machineSet.Name, annotations) + klog.V(4).Infof("De-annotated the node %q backed by MachineSet %q with %s", machine.Labels[v1alpha1.NodeLabelKey], machineSet.Name, annotations) } } } diff --git a/pkg/controller/deployment_rolling_test.go b/pkg/controller/deployment_rolling_test.go index 04d18c430..b75747e73 100644 --- a/pkg/controller/deployment_rolling_test.go +++ b/pkg/controller/deployment_rolling_test.go @@ -130,7 +130,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -293,7 +293,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -345,7 +345,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -396,7 +396,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -447,7 +447,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 0, @@ -589,7 +589,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -643,7 +643,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, @@ -697,7 +697,7 @@ var _ = Describe("deployment_rolling", func() { machineSets[0], &machinev1.MachineStatus{}, nil, - map[string]string{machinev1.MachineNodeLabelKey: "node-0"}, + map[string]string{machinev1.NodeLabelKey: "node-0"}, ), nodes: newNodes( 1, diff --git a/pkg/controller/deployment_test.go b/pkg/controller/deployment_test.go index 72c04eaa1..0f547ccb6 100644 --- a/pkg/controller/deployment_test.go +++ b/pkg/controller/deployment_test.go @@ -1514,8 +1514,8 @@ var _ = Describe("machineDeployment", func() { Name: "Machine-test", Namespace: testNamespace, Labels: map[string]string{ - "test-label": "test-label", - machinev1.MachineNodeLabelKey: "Node1-test", + "test-label": "test-label", + machinev1.NodeLabelKey: "Node1-test", }, UID: "1234567", OwnerReferences: []metav1.OwnerReference{ diff --git a/pkg/controller/machine.go b/pkg/controller/machine.go index 98e5ff6b8..d8c9172f9 100644 --- a/pkg/controller/machine.go +++ b/pkg/controller/machine.go @@ -197,7 +197,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp } // Sync nodeTemplate between machine and node-objects. - node, _ := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) + node, _ := c.nodeLister.Get(machine.Labels[v1alpha1.NodeLabelKey]) if node != nil { err = c.syncMachineNodeTemplates(ctx, machine) if err != nil { @@ -297,7 +297,7 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err var ( list = []string{nodeName} selector = labels.NewSelector() - req, _ = labels.NewRequirement(v1alpha1.MachineNodeLabelKey, selection.Equals, list) + req, _ = labels.NewRequirement(v1alpha1.NodeLabelKey, selection.Equals, list) ) selector = selector.Add(*req) @@ -315,7 +315,7 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.Machine) (*v1alpha1.Machine, error) { nodeName := "" if machine.Labels != nil { - nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName = machine.Labels[v1alpha1.NodeLabelKey] } if nodeName == "" { @@ -338,7 +338,7 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M if clone.Labels == nil { clone.Labels = make(map[string]string) } - clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName + clone.Labels[v1alpha1.NodeLabelKey] = nodeName machine, err = c.controlMachineClient.Machines(clone.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) if err != nil { klog.Errorf("Could not update the machine-object %s due to error %v", machine.Name, err) @@ -530,7 +530,7 @@ func (c *controller) machineCreate(ctx context.Context, machine *v1alpha1.Machin if clone.Labels == nil { clone.Labels = make(map[string]string) } - clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName + clone.Labels[v1alpha1.NodeLabelKey] = nodeName if clone.Annotations == nil { clone.Annotations = make(map[string]string) @@ -600,7 +600,7 @@ func (c *controller) machineUpdate(ctx context.Context, machine *v1alpha1.Machin func (c *controller) machineDelete(ctx context.Context, machine *v1alpha1.Machine, driver driver.Driver) error { var err error - nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName := machine.Labels[v1alpha1.NodeLabelKey] if finalizers := sets.NewString(machine.Finalizers...); finalizers.Has(DeleteFinalizerName) { klog.V(2).Infof("Deleting Machine %q", machine.Name) diff --git a/pkg/controller/machine_test.go b/pkg/controller/machine_test.go index bd7ce4c7e..0af146f62 100644 --- a/pkg/controller/machine_test.go +++ b/pkg/controller/machine_test.go @@ -707,7 +707,7 @@ var _ = Describe("machine", func() { Expect(err).To(BeNil()) Expect(actual.Spec).To(Equal(data.expect.machine.Spec)) Expect(actual.Labels).ToNot(BeNil()) - Expect(actual.Labels[v1alpha1.MachineNodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey])) + Expect(actual.Labels[v1alpha1.NodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.NodeLabelKey])) //TODO Conditions }, Entry("OpenStackSimple", &data{ @@ -804,7 +804,7 @@ var _ = Describe("machine", func() { }, }, &machinev1.MachineStatus{ //TODO conditions - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "fakeNode-0"}), err: false, }, }), @@ -864,7 +864,7 @@ var _ = Describe("machine", func() { }, }, &machinev1.MachineStatus{ //TODO conditions - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "fakeNode-0"}), err: false, }, }), @@ -972,7 +972,7 @@ var _ = Describe("machine", func() { }, }, &machinev1.MachineStatus{ //TODO conditions - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: ""}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: ""}), err: false, }, }), @@ -1095,7 +1095,7 @@ var _ = Describe("machine", func() { } if data.action.nodeRecentlyNotReady != nil { - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(Not(HaveOccurred())) clone := node.DeepCopy() newNodeCondition := corev1.NodeCondition{ @@ -1126,7 +1126,7 @@ var _ = Describe("machine", func() { Expect(err).ToNot(HaveOccurred()) } - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) machine, machineErr := controller.controlMachineClient.Machines(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{}) if data.expect.machineDeleted { @@ -1902,8 +1902,8 @@ var _ = Describe("machine", func() { Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) if data.expect.machine.Labels != nil { - if _, ok := data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey]; ok { - Expect(actual.Labels[v1alpha1.MachineNodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.MachineNodeLabelKey])) + if _, ok := data.expect.machine.Labels[v1alpha1.NodeLabelKey]; ok { + Expect(actual.Labels[v1alpha1.NodeLabelKey]).To(Equal(data.expect.machine.Labels[v1alpha1.NodeLabelKey])) } } @@ -1933,7 +1933,7 @@ var _ = Describe("machine", func() { setup: setup{ machines: newMachines(1, &machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "dummy-node"}), }, action: action{ machine: machineName, @@ -1941,7 +1941,7 @@ var _ = Describe("machine", func() { expect: expect{ machine: newMachine(&machinev1.MachineTemplateSpec{ ObjectMeta: *newObjectMeta(objMeta, 0), - }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "dummy-node"}), }, }), Entry("Machine is running but node object is lost", &data{ @@ -1971,7 +1971,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "dummy-node"}), }, action: action{ machine: machineName, @@ -2002,7 +2002,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "dummy-node"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "dummy-node"}), }, }), Entry("Machine and node both are present and kubelet ready status is updated", &data{ @@ -2029,7 +2029,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}), nodes: []*corev1.Node{ { TypeMeta: metav1.TypeMeta{ @@ -2078,7 +2078,7 @@ var _ = Describe("machine", func() { Type: "Ready", }, }, - }, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), + }, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}), }, }), Entry("Machine object does not have node label set and node exists then it should adopt node using providerID", &data{ @@ -2108,7 +2108,7 @@ var _ = Describe("machine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "machine-0", }, - }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}), + }, &machinev1.MachineStatus{}, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}), }, }), Entry("Machine object does not have node label set and node exists (without providerID) then it should not adopt node using providerID", &data{ diff --git a/pkg/controller/machine_util.go b/pkg/controller/machine_util.go index e1eb8042b..b11e6ee1d 100644 --- a/pkg/controller/machine_util.go +++ b/pkg/controller/machine_util.go @@ -303,7 +303,7 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al currentlyAppliedALTJSONByte []byte ) - node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.NodeLabelKey]) if err != nil || node == nil { klog.Errorf("Error: Could not get the node-object or node-object is missing - err: %q", err) // Dont return error so that other steps can be executed. @@ -516,7 +516,7 @@ func (c *controller) UpdateNodeTerminationCondition(ctx context.Context, machine return nil } - nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName := machine.Labels[v1alpha1.NodeLabelKey] terminationCondition := v1.NodeCondition{ Type: NodeTerminationCondition, Status: v1.ConditionTrue, diff --git a/pkg/controller/machine_util_test.go b/pkg/controller/machine_util_test.go index b141a2e8b..b731a2877 100644 --- a/pkg/controller/machine_util_test.go +++ b/pkg/controller/machine_util_test.go @@ -127,7 +127,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -222,7 +222,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -301,7 +301,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{}, @@ -353,7 +353,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -1909,7 +1909,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineFailed}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -1954,7 +1954,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineRunning}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -1999,7 +1999,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2044,7 +2044,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2093,7 +2093,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineFailed}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{ @@ -2137,7 +2137,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineStatus{ CurrentStatus: machinev1.CurrentStatus{Phase: MachineTerminating}, }, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}), }, action: action{ node: &corev1.Node{}, diff --git a/pkg/util/provider/machinecontroller/machine.go b/pkg/util/provider/machinecontroller/machine.go index 999588133..a09e9a136 100644 --- a/pkg/util/provider/machinecontroller/machine.go +++ b/pkg/util/provider/machinecontroller/machine.go @@ -161,7 +161,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } - if machine.Labels[v1alpha1.MachineNodeLabelKey] != "" { + if machine.Labels[v1alpha1.NodeLabelKey] != "" { // If reference to node object exists execute the below retry, err := c.reconcileMachineHealth(ctx, machine) if err != nil { @@ -173,7 +173,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } } - if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" || machine.Labels[v1alpha1.MachineNodeLabelKey] == "" { + if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" || machine.Labels[v1alpha1.NodeLabelKey] == "" { return c.triggerCreationFlow( ctx, &driver.CreateMachineRequest{ @@ -349,7 +349,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque // Either VM is not found // or GetMachineStatus() call is not implemented // In this case, invoke a CreateMachine() call - if _, present := machine.Labels[v1alpha1.MachineNodeLabelKey]; !present { + if _, present := machine.Labels[v1alpha1.NodeLabelKey]; !present { // If node label is not present klog.V(2).Infof("Creating a VM for machine %q, please wait!", machine.Name) klog.V(2).Infof("The machine creation is triggered with timeout of %s", c.getEffectiveCreationTimeout(createMachineRequest.Machine).Duration) @@ -417,7 +417,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque } } else { // if node label present that means there must be a backing VM ,without need of GetMachineStatus() call - nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName = machine.Labels[v1alpha1.NodeLabelKey] } case codes.Unknown, codes.DeadlineExceeded, codes.Aborted, codes.Unavailable: @@ -461,14 +461,14 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque return machineutils.MediumRetry, err } } else { - if machine.Labels[v1alpha1.MachineNodeLabelKey] == "" || machine.Spec.ProviderID == "" { + if machine.Labels[v1alpha1.NodeLabelKey] == "" || machine.Spec.ProviderID == "" { klog.V(2).Infof("Found VM with required machine name. Adopting existing machine: %q with ProviderID: %s", machineName, getMachineStatusResponse.ProviderID) } nodeName = getMachineStatusResponse.NodeName providerID = getMachineStatusResponse.ProviderID } - _, machineNodeLabelPresent := createMachineRequest.Machine.Labels[v1alpha1.MachineNodeLabelKey] + _, machineNodeLabelPresent := createMachineRequest.Machine.Labels[v1alpha1.NodeLabelKey] _, machinePriorityAnnotationPresent := createMachineRequest.Machine.Annotations[machineutils.MachinePriority] if !machineNodeLabelPresent || !machinePriorityAnnotationPresent || machine.Spec.ProviderID == "" { @@ -476,7 +476,7 @@ func (c *controller) triggerCreationFlow(ctx context.Context, createMachineReque if clone.Labels == nil { clone.Labels = make(map[string]string) } - clone.Labels[v1alpha1.MachineNodeLabelKey] = nodeName + clone.Labels[v1alpha1.NodeLabelKey] = nodeName if clone.Annotations == nil { clone.Annotations = make(map[string]string) } diff --git a/pkg/util/provider/machinecontroller/machine_safety_test.go b/pkg/util/provider/machinecontroller/machine_safety_test.go index 97782a982..fabea8d22 100644 --- a/pkg/util/provider/machinecontroller/machine_safety_test.go +++ b/pkg/util/provider/machinecontroller/machine_safety_test.go @@ -447,7 +447,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_1", Namespace: testNamespace, Labels: map[string]string{ - v1alpha1.MachineNodeLabelKey: "test-node-1", + v1alpha1.NodeLabelKey: "test-node-1", }, }, Status: v1alpha1.MachineStatus{ @@ -464,7 +464,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_2", Namespace: testNamespace, Labels: map[string]string{ - v1alpha1.MachineNodeLabelKey: "test-node-2", + v1alpha1.NodeLabelKey: "test-node-2", }, }, Status: v1alpha1.MachineStatus{ @@ -481,7 +481,7 @@ var _ = Describe("safety_logic", func() { Name: "testmachine_3", Namespace: testNamespace, Labels: map[string]string{ - v1alpha1.MachineNodeLabelKey: "test-node-2", + v1alpha1.NodeLabelKey: "test-node-2", }, }, Status: v1alpha1.MachineStatus{ diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go index 1c45d56f9..209d05080 100644 --- a/pkg/util/provider/machinecontroller/machine_test.go +++ b/pkg/util/provider/machinecontroller/machine_test.go @@ -122,7 +122,7 @@ var _ = Describe("machine", func() { Entry("with NodeReady is Unknown", corev1.NodeReady, corev1.ConditionUnknown, false), ) }) - + Describe("#ValidateMachine", func() { type data struct { action machineapi.Machine @@ -447,7 +447,7 @@ var _ = Describe("machine", func() { }, ProviderID: "fakeID", }, - }, nil, nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "fakeNode-0"}, true, metav1.Now()), + }, nil, nil, nil, map[string]string{v1alpha1.NodeLabelKey: "fakeNode-0"}, true, metav1.Now()), err: fmt.Errorf("Machine creation in process. Machine UPDATE successful"), retry: machineutils.ShortRetry, }, @@ -484,7 +484,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -521,7 +521,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -573,7 +573,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -618,7 +618,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -946,11 +946,11 @@ var _ = Describe("machine", func() { Expect(machine.Finalizers).To(Equal(data.expect.machine.Finalizers)) if data.expect.nodeDeleted { - _, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + _, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(HaveOccurred()) } if data.expect.nodeTerminationConditionIsSet { - node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.MachineNodeLabelKey], metav1.GetOptions{}) + node, nodeErr := controller.targetCoreClient.CoreV1().Nodes().Get(context.TODO(), machine.Labels[v1alpha1.NodeLabelKey], metav1.GetOptions{}) Expect(nodeErr).To(Not(HaveOccurred())) Expect(len(node.Status.Conditions)).To(Equal(1)) Expect(node.Status.Conditions[0].Type).To(Equal(machineutils.NodeTerminationCondition)) @@ -1000,7 +1000,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -1046,7 +1046,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -1095,7 +1095,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1141,7 +1141,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1190,7 +1190,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1236,7 +1236,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1285,7 +1285,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeNode-0", + v1alpha1.NodeLabelKey: "fakeNode-0", }, true, metav1.Now(), @@ -1339,7 +1339,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1395,7 +1395,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "", + v1alpha1.NodeLabelKey: "", }, true, metav1.Now(), @@ -1441,7 +1441,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "", + v1alpha1.NodeLabelKey: "", }, true, metav1.Now(), @@ -1497,7 +1497,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1543,7 +1543,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1599,7 +1599,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1645,7 +1645,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1706,7 +1706,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1752,7 +1752,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1808,7 +1808,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1854,7 +1854,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1910,7 +1910,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -1956,7 +1956,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2005,8 +2005,8 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", - "force-deletion": "True", + v1alpha1.NodeLabelKey: "fakeID-0", + "force-deletion": "True", }, true, metav1.Now(), @@ -2064,7 +2064,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2113,7 +2113,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.NewTime(time.Now().Add(-3*time.Minute)), @@ -2171,7 +2171,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2220,7 +2220,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.NewTime(time.Now().Add(-3*time.Hour)), @@ -2278,7 +2278,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2327,7 +2327,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeNode-0", + v1alpha1.NodeLabelKey: "fakeNode-0", }, true, metav1.Now(), @@ -2385,7 +2385,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2434,7 +2434,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2480,7 +2480,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2529,7 +2529,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2583,7 +2583,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2632,7 +2632,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2677,7 +2677,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, false, metav1.Now(), @@ -2726,7 +2726,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), @@ -2772,7 +2772,7 @@ var _ = Describe("machine", func() { machineutils.MachinePriority: "3", }, map[string]string{ - v1alpha1.MachineNodeLabelKey: "fakeID-0", + v1alpha1.NodeLabelKey: "fakeID-0", }, true, metav1.Now(), diff --git a/pkg/util/provider/machinecontroller/machine_util.go b/pkg/util/provider/machinecontroller/machine_util.go index 10a312c0c..e48ea8fa7 100644 --- a/pkg/util/provider/machinecontroller/machine_util.go +++ b/pkg/util/provider/machinecontroller/machine_util.go @@ -256,7 +256,7 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al lastAppliedALT v1alpha1.NodeTemplateSpec ) - node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.NodeLabelKey]) if err != nil && apierrors.IsNotFound(err) { // Dont return error so that other steps can be executed. return machineutils.LongRetry, nil @@ -589,7 +589,7 @@ func (c *controller) reconcileMachineHealth(ctx context.Context, machine *v1alph lastOperationType v1alpha1.MachineOperationType ) - node, err := c.nodeLister.Get(machine.Labels[v1alpha1.MachineNodeLabelKey]) + node, err := c.nodeLister.Get(machine.Labels[v1alpha1.NodeLabelKey]) if err != nil { if apierrors.IsNotFound(err) { // Node object is not found @@ -987,7 +987,7 @@ func (c *controller) drainNode(ctx context.Context, deleteMachineRequest *driver pvReattachTimeOut = c.safetyOptions.PvReattachTimeout.Duration timeOutDuration = c.getEffectiveDrainTimeout(deleteMachineRequest.Machine).Duration forceDeleteLabelPresent = machine.Labels["force-deletion"] == "True" - nodeName = machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName = machine.Labels[v1alpha1.NodeLabelKey] nodeNotReadyDuration = 5 * time.Minute ReadonlyFilesystem v1.NodeConditionType = "ReadonlyFilesystem" ) @@ -1213,7 +1213,7 @@ func (c *controller) deleteNodeObject(ctx context.Context, machine *v1alpha1.Mac state v1alpha1.MachineState ) - nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName := machine.Labels[v1alpha1.NodeLabelKey] if nodeName != "" { // Delete node object @@ -1318,7 +1318,7 @@ func (c *controller) UpdateNodeTerminationCondition(ctx context.Context, machine return nil } - nodeName := machine.Labels[v1alpha1.MachineNodeLabelKey] + nodeName := machine.Labels[v1alpha1.NodeLabelKey] terminationCondition := v1.NodeCondition{ Type: machineutils.NodeTerminationCondition, @@ -1515,7 +1515,7 @@ func getProviderID(machine *v1alpha1.Machine) string { } func getNodeName(machine *v1alpha1.Machine) string { - return machine.Labels[v1alpha1.MachineNodeLabelKey] + return machine.Labels[v1alpha1.NodeLabelKey] } func getMachineDeploymentName(machine *v1alpha1.Machine) string { diff --git a/pkg/util/provider/machinecontroller/machine_util_test.go b/pkg/util/provider/machinecontroller/machine_util_test.go index b368ffcca..606c5a62e 100644 --- a/pkg/util/provider/machinecontroller/machine_util_test.go +++ b/pkg/util/provider/machinecontroller/machine_util_test.go @@ -143,7 +143,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -238,7 +238,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -317,7 +317,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{}, @@ -369,7 +369,7 @@ var _ = Describe("machine_util", func() { }, }, &machinev1.MachineStatus{}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "test-node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "test-node-0"}, true, metav1.Now()), }, action: action{ node: &corev1.Node{ @@ -2172,7 +2172,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-25 * time.Minute))}}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2188,7 +2188,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.Now()}}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, false, false, false, false)}), @@ -2207,7 +2207,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2226,7 +2226,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, true, false, false, false)}), @@ -2245,7 +2245,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineRunning, LastUpdateTime: metav1.Now()}, Conditions: nodeConditions(true, false, false, false, false)}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2262,7 +2262,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2281,7 +2281,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, targetMachineName: machineSet1Deploy1 + "-" + "0", }, @@ -2296,7 +2296,7 @@ var _ = Describe("machine_util", func() { newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(true, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, - nil, nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0-0"}, true, metav1.Now()), + nil, nil, map[string]string{v1alpha1.NodeLabelKey: "node-0-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(true, false, false, false, false)}), @@ -2374,12 +2374,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy2}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2398,12 +2398,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy2}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2421,7 +2421,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), }, @@ -2439,12 +2439,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineFailed, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2464,12 +2464,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineTerminating, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2489,12 +2489,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: "", LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2514,12 +2514,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2539,12 +2539,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 1)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineCrashLoopBackOff, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2564,12 +2564,12 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), newMachine( &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet2Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachinePending, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), @@ -2589,7 +2589,7 @@ var _ = Describe("machine_util", func() { &machinev1.MachineTemplateSpec{ObjectMeta: *newObjectMeta(&metav1.ObjectMeta{GenerateName: machineSet1Deploy1}, 0)}, &machinev1.MachineStatus{Conditions: nodeConditions(false, false, false, false, false), CurrentStatus: machinev1.CurrentStatus{Phase: v1alpha1.MachineUnknown, LastUpdateTime: metav1.NewTime(time.Now().Add(-15 * time.Minute))}}, &metav1.OwnerReference{Name: machineSet1Deploy1}, - nil, map[string]string{"name": machineDeploy1, v1alpha1.MachineNodeLabelKey: "node-0"}, true, metav1.Now()), + nil, map[string]string{"name": machineDeploy1, v1alpha1.NodeLabelKey: "node-0"}, true, metav1.Now()), }, nodes: []*v1.Node{ newNode(1, nil, nil, &v1.NodeSpec{}, &v1.NodeStatus{Phase: corev1.NodeRunning, Conditions: nodeConditions(false, false, false, false, false)}), From 17fafd946746b0f05fdc687658dc50bbf1e69ac8 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Thu, 5 Jan 2023 16:29:06 +0530 Subject: [PATCH 3/3] addressing review comments --- pkg/controller/machine.go | 22 +- .../provider/machinecontroller/machine.go | 12 +- .../machinecontroller/machine_test.go | 544 +++++++++++++++++- 3 files changed, 559 insertions(+), 19 deletions(-) diff --git a/pkg/controller/machine.go b/pkg/controller/machine.go index d8c9172f9..20d7b4640 100644 --- a/pkg/controller/machine.go +++ b/pkg/controller/machine.go @@ -58,8 +58,8 @@ const ( ) /* - SECTION - Machine controller - Machine add, update, delete watches +SECTION +Machine controller - Machine add, update, delete watches */ func (c *controller) addMachine(obj interface{}) { klog.V(4).Infof("Adding machine object") @@ -236,8 +236,8 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp } /* - SECTION - Machine controller - nodeToMachine +SECTION +Machine controller - nodeToMachine */ func (c *controller) addNodeToMachine(obj interface{}) { node := obj.(*corev1.Node) @@ -314,9 +314,7 @@ func (c *controller) getMachineFromNode(nodeName string) (*v1alpha1.Machine, err func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.Machine) (*v1alpha1.Machine, error) { nodeName := "" - if machine.Labels != nil { - nodeName = machine.Labels[v1alpha1.NodeLabelKey] - } + nodeName = machine.Labels[v1alpha1.NodeLabelKey] if nodeName == "" { // Check if any existing node-object can be adopted. @@ -339,10 +337,10 @@ func (c *controller) updateMachineState(ctx context.Context, machine *v1alpha1.M clone.Labels = make(map[string]string) } clone.Labels[v1alpha1.NodeLabelKey] = nodeName - machine, err = c.controlMachineClient.Machines(clone.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) + clone, err = c.controlMachineClient.Machines(clone.Namespace).Update(ctx, clone, metav1.UpdateOptions{}) if err != nil { - klog.Errorf("Could not update the machine-object %s due to error %v", machine.Name, err) - return machine, err + klog.Errorf("Could not update the node label for machine-object %s due to error %v", machine.Name, err) + return clone, err } break } @@ -998,8 +996,8 @@ func (c *controller) deleteMachineFinalizers(ctx context.Context, machine *v1alp } /* - SECTION - Helper Functions +SECTION +Helper Functions */ func (c *controller) isHealthy(machine *v1alpha1.Machine) bool { numOfConditions := len(machine.Status.Conditions) diff --git a/pkg/util/provider/machinecontroller/machine.go b/pkg/util/provider/machinecontroller/machine.go index a09e9a136..571c03d7c 100644 --- a/pkg/util/provider/machinecontroller/machine.go +++ b/pkg/util/provider/machinecontroller/machine.go @@ -42,8 +42,8 @@ import ( ) /* - SECTION - Machine controller - Machine add, update, delete watches +SECTION +Machine controller - Machine add, update, delete watches */ func (c *controller) addMachine(obj interface{}) { klog.V(5).Infof("Adding machine object") @@ -161,7 +161,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } - if machine.Labels[v1alpha1.NodeLabelKey] != "" { + if machine.Labels[v1alpha1.NodeLabelKey] != "" && machine.Status.CurrentStatus.Phase != "" { // If reference to node object exists execute the below retry, err := c.reconcileMachineHealth(ctx, machine) if err != nil { @@ -173,7 +173,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp return retry, err } } - if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" || machine.Labels[v1alpha1.NodeLabelKey] == "" { + if machine.Spec.ProviderID == "" || machine.Status.CurrentStatus.Phase == "" { return c.triggerCreationFlow( ctx, &driver.CreateMachineRequest{ @@ -188,8 +188,8 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp } /* - SECTION - Machine controller - nodeToMachine +SECTION +Machine controller - nodeToMachine */ var ( errMultipleMachineMatch = errors.New("Multiple machines matching node") diff --git a/pkg/util/provider/machinecontroller/machine_test.go b/pkg/util/provider/machinecontroller/machine_test.go index 209d05080..ad643d3f9 100644 --- a/pkg/util/provider/machinecontroller/machine_test.go +++ b/pkg/util/provider/machinecontroller/machine_test.go @@ -5,7 +5,7 @@ Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at - http://www.apache.org/licenses/LICENSE-2.0 + http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, @@ -123,6 +123,82 @@ var _ = Describe("machine", func() { ) }) + /* + Describe("##updateMachineConditions", func() { + Describe("Update conditions of a non-existing machine", func() { + It("should return error", func() { + stop := make(chan struct{}) + defer close(stop) + + objects := []runtime.Object{} + c, trackers := createController(stop, testNamespace, objects, nil, nil) + defer trackers.Stop() + + testMachine := &v1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testmachine", + Namespace: testNamespace, + }, + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineTerminating, + }, + }, + } + conditions := []corev1.NodeCondition{} + var _, err = c.updateMachineConditions(testMachine, conditions) + Expect(err).Should(Not(BeNil())) + }) + }) + DescribeTable("Update conditions of an existing machine", + func(phase v1alpha1.MachinePhase, conditions []corev1.NodeCondition, expectedPhase v1alpha1.MachinePhase) { + stop := make(chan struct{}) + defer close(stop) + + testMachine := &v1alpha1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testmachine", + Namespace: testNamespace, + }, + Status: v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: phase, + }, + }, + } + objects := []runtime.Object{} + objects = append(objects, testMachine) + + c, trackers := createController(stop, testNamespace, objects, nil, nil) + defer trackers.Stop() + + var updatedMachine, err = c.updateMachineConditions(testMachine, conditions) + Expect(updatedMachine.Status.Conditions).Should(BeEquivalentTo(conditions)) + Expect(updatedMachine.Status.CurrentStatus.Phase).Should(BeIdenticalTo(expectedPhase)) + Expect(err).Should(BeNil()) + }, + Entry("healthy status but machine terminating", v1alpha1.MachineTerminating, []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + }, + }, v1alpha1.MachineTerminating), + Entry("unhealthy status but machine running", v1alpha1.MachineRunning, []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionFalse, + }, + }, v1alpha1.MachineUnknown), + Entry("healthy status but machine not running", v1alpha1.MachineAvailable, []corev1.NodeCondition{ + { + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + }, + }, v1alpha1.MachineRunning), + ) + }) + */ + Describe("#ValidateMachine", func() { type data struct { action machineapi.Machine @@ -2782,4 +2858,470 @@ var _ = Describe("machine", func() { ) }) + /* + Describe("#checkMachineTimeout", func() { + type setup struct { + machines []*v1alpha1.Machine + } + type action struct { + machine string + } + type expect struct { + machine *v1alpha1.Machine + err bool + } + type data struct { + setup setup + action action + expect expect + } + objMeta := &metav1.ObjectMeta{ + GenerateName: "machine", + Namespace: "test", + } + machineName := "machine-0" + timeOutOccurred := -21 * time.Minute + timeOutNotOccurred := -5 * time.Minute + creationTimeOut := 20 * time.Minute + healthTimeOut := 10 * time.Minute + DescribeTable("##Machine Timeout Scenarios", + func(data *data) { + stop := make(chan struct{}) + defer close(stop) + machineObjects := []runtime.Object{} + for _, o := range data.setup.machines { + machineObjects = append(machineObjects, o) + } + coreObjects := []runtime.Object{} + controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects) + defer trackers.Stop() + waitForCacheSync(stop, controller) + action := data.action + machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) + //Expect(err).ToNot(HaveOccurred()) + controller.checkMachineTimeout(machine) + actual, err := controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{}) + Expect(err).To(BeNil()) + Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) + Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) + Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) + Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) + Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) + }, + Entry("Machine is still running", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), + }, + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + }, + }, nil, nil, nil), + }, + }), + Entry("Machine creation has still not timed out", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutNotOccurred)), + }, + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + }, + }, nil, nil, nil), + }, + }), + Entry("Machine creation has timed out", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Creating machine on cloud provider", + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), + }, + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineFailed, + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf( + "Machine %s failed to join the cluster in %s minutes.", + machineName, + creationTimeOut, + ), + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationCreate, + }, + }, nil, nil, nil), + }, + }), + Entry("Machine health has timed out", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine %s is unhealthy - changing MachineState to Unknown", machineName), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationHealthCheck, + LastUpdateTime: metav1.NewTime(time.Now().Add(timeOutOccurred)), + }, + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineFailed, + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf( + "Machine %s is not healthy since %s minutes. Changing status to failed. Node Conditions: %+v", + machineName, + healthTimeOut, + []corev1.NodeCondition{}, + ), + State: v1alpha1.MachineStateFailed, + Type: v1alpha1.MachineOperationHealthCheck, + }, + }, nil, nil, nil), + }, + }), + ) + }) + Describe("#updateMachineState", func() { + type setup struct { + machines []*v1alpha1.Machine + nodes []*corev1.Node + } + type action struct { + machine string + } + type expect struct { + machine *v1alpha1.Machine + err bool + } + type data struct { + setup setup + action action + expect expect + } + objMeta := &metav1.ObjectMeta{ + GenerateName: "machine", + // using default namespace for non-namespaced objects + // as our current fake client is with the assumption + // that all objects are namespaced + Namespace: "", + } + machineName := "machine-0" + DescribeTable("##Different machine state update scenrios", + func(data *data) { + stop := make(chan struct{}) + defer close(stop) + machineObjects := []runtime.Object{} + for _, o := range data.setup.machines { + machineObjects = append(machineObjects, o) + } + coreObjects := []runtime.Object{} + for _, o := range data.setup.nodes { + coreObjects = append(coreObjects, o) + } + controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects) + defer trackers.Stop() + waitForCacheSync(stop, controller) + action := data.action + machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + controller.updateMachineState(machine) + actual, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{}) + Expect(err).To(BeNil()) + Expect(actual.Name).To(Equal(data.expect.machine.Name)) + Expect(actual.Status.Node).To(Equal(data.expect.machine.Status.Node)) + Expect(actual.Status.CurrentStatus.Phase).To(Equal(data.expect.machine.Status.CurrentStatus.Phase)) + Expect(actual.Status.CurrentStatus.//TimeoutActive).To(Equal(data.expect.machine.Status.CurrentStatus.//TimeoutActive)) + Expect(actual.Status.LastOperation.State).To(Equal(data.expect.machine.Status.LastOperation.State)) + Expect(actual.Status.LastOperation.Type).To(Equal(data.expect.machine.Status.LastOperation.Type)) + Expect(actual.Status.LastOperation.Description).To(Equal(data.expect.machine.Status.LastOperation.Description)) + if data.expect.machine.Labels != nil { + if _, ok := data.expect.machine.Labels["node"]; ok { + Expect(actual.Labels["node"]).To(Equal(data.expect.machine.Labels["node"])) + } + } + for i := range actual.Status.Conditions { + Expect(actual.Status.Conditions[i].Type).To(Equal(data.expect.machine.Status.Conditions[i].Type)) + Expect(actual.Status.Conditions[i].Status).To(Equal(data.expect.machine.Status.Conditions[i].Status)) + Expect(actual.Status.Conditions[i].Reason).To(Equal(data.expect.machine.Status.Conditions[i].Reason)) + Expect(actual.Status.Conditions[i].Message).To(Equal(data.expect.machine.Status.Conditions[i].Message)) + } + }, + Entry("Machine does not have a node backing", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{}, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{}, nil, nil, nil), + }, + }), + Entry("Node object backing machine not found and machine conditions are empty", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "dummy-node", + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "dummy-node", + }, nil, nil, nil), + }, + }), + Entry("Machine is running but node object is lost", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "dummy-node", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf("Machine % successfully joined the cluster", machineName), + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + Conditions: []corev1.NodeCondition{ + { + Message: "kubelet is posting ready status", + Reason: "KubeletReady", + Status: "True", + Type: "Ready", + }, + }, + }, nil, nil, nil), + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "dummy-node", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineUnknown, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: fmt.Sprintf( + "Node object went missing. Machine %s is unhealthy - changing MachineState to Unknown", + machineName, + ), + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationHealthCheck, + LastUpdateTime: metav1.Now(), + }, + Conditions: []corev1.NodeCondition{ + { + Message: "kubelet is posting ready status", + Reason: "KubeletReady", + Status: "True", + Type: "Ready", + }, + }, + }, nil, nil, nil), + }, + }), + Entry("Machine and node both are present and kubelet ready status is updated", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "machine", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachinePending, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Creating machine on cloud provider", + State: v1alpha1.MachineStateProcessing, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + Conditions: []corev1.NodeCondition{ + { + Message: "kubelet is not ready", + Reason: "KubeletReady", + Status: "False", + Type: "Ready", + }, + }, + }, nil, nil, nil), + nodes: []*corev1.Node{ + { + ObjectMeta: *newObjectMeta(objMeta, 0), + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{ + { + Message: "kubelet is posting ready status", + Reason: "KubeletReady", + Status: "True", + Type: "Ready", + }, + }, + }, + }, + }, + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "machine", + CurrentStatus: v1alpha1.CurrentStatus{ + Phase: v1alpha1.MachineRunning, + //TimeoutActive: false, + LastUpdateTime: metav1.Now(), + }, + LastOperation: v1alpha1.LastOperation{ + Description: "Machine machine-0 successfully joined the cluster", + State: v1alpha1.MachineStateSuccessful, + Type: v1alpha1.MachineOperationCreate, + LastUpdateTime: metav1.Now(), + }, + Conditions: []corev1.NodeCondition{ + { + Message: "kubelet is posting ready status", + Reason: "KubeletReady", + Status: "True", + Type: "Ready", + }, + }, + }, nil, nil, nil), + }, + }), + Entry("Machine object does not have node-label and node exists", &data{ + setup: setup{ + machines: newMachines(1, &v1alpha1.MachineTemplateSpec{ + ObjectMeta: *newObjectMeta(objMeta, 0), + }, &v1alpha1.MachineStatus{ + Node: "node", + }, nil, nil, nil), + nodes: []*corev1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node-0", + }, + }, + }, + }, + action: action{ + machine: machineName, + }, + expect: expect{ + machine: newMachine(&v1alpha1.MachineTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine-0", + }, + }, &v1alpha1.MachineStatus{ + Node: "node", + }, nil, nil, + map[string]string{ + "node": "node-0", + }, + ), + }, + }), + ) + }) + */ })