Add implementation for NodeUpgradeController (#7061)

* Add implementation for NodeUpgradeController

* Add conditions to NodeUpgrader

* Fix linter and add unit tests

* improve codecov

* cleanup spec objects

* Add some TODOs for cleanup

* fix linter

* Add more coverage

* resolve comments

Makefile

@@ -603,6 +603,7 @@ mocks: ## Generate mocks
 	${MOCKGEN} -destination=pkg/controller/clusters/mocks/ipvalidator.go -package=mocks -source "pkg/controller/clusters/ipvalidator.go" IPUniquenessValidator
 	${MOCKGEN} -destination=pkg/registry/mocks/storage.go -package=mocks -source "pkg/registry/storage.go" StorageClient
 	${MOCKGEN} -destination=pkg/registry/mocks/repository.go -package=mocks oras.land/oras-go/v2/registry Repository
+	${MOCKGEN} -destination=controllers/mocks/nodeupgrade_controller.go -package=mocks -source "controllers/nodeupgrade_controller.go" RemoteClientRegistry
 
 .PHONY: verify-mocks
 verify-mocks: mocks ## Verify if mocks need to be updated

config/crd/bases/anywhere.eks.amazonaws.com_nodeupgrades.yaml

@@ -40,38 +40,60 @@ spec:
                 type: string
               etcdVersion:
                 type: string
-              kubeletVersion:
-                type: string
+              firstNodeToBeUpgraded:
+                description: FirstNodeToBeUpgraded signifies that the Node is the
+                  first node to be upgraded. This flag is only valid for control plane
+                  nodes and ignored for worker nodes.
+                type: boolean
               kubernetesVersion:
                 type: string
               machine:
+                description: Machine is a reference to the CAPI Machine that needs
+                  to be upgraded.
                 properties:
-                  kind:
+                  apiVersion:
+                    description: API version of the referent.
                     type: string
-                  name:
+                  fieldPath:
+                    description: 'If referring to a piece of an object instead of
+                      an entire object, this string should contain a valid JSON/Go
+                      field access statement, such as desiredState.manifest.containers[2].
+                      For example, if the object reference is to a container within
+                      a pod, this would take on a value like: "spec.containers{name}"
+                      (where "name" refers to the name of the container that triggered
+                      the event) or if no container name is specified "spec.containers[2]"
+                      (container with index 2 in this pod). This syntax is chosen
+                      only to have some well-defined way of referencing a part of
+                      an object. TODO: this design is not final and this field is
+                      subject to change in the future.'
                     type: string
-                type: object
-              node:
-                properties:
                   kind:
+                    description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
                     type: string
                   name:
+                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
+                    type: string
+                  namespace:
+                    description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
+                    type: string
+                  resourceVersion:
+                    description: 'Specific resourceVersion to which this reference
+                      is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
+                    type: string
+                  uid:
+                    description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
                     type: string
                 type: object
             required:
-            - kubeletVersion
             - kubernetesVersion
             - machine
-            - node
             type: object
           status:
             description: NodeUpgradeStatus defines the observed state of NodeUpgrade.
             properties:
               completed:
                 type: boolean
               conditions:
-                description: Conditions provide observations of the operational state
-                  of a Cluster API resource.
                 items:
                   description: Condition defines an observation of a Cluster API resource
                     operational state.
@@ -115,11 +137,11 @@ spec:
                   - type
                   type: object
                 type: array
-              phase:
-                type: string
-            required:
-            - completed
-            - phase
+              observedGeneration:
+                description: ObservedGeneration is the latest generation observed
+                  by the controller.
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

config/manifest/eksa-components.yaml

@@ -4893,38 +4893,60 @@ spec:
                 type: string
               etcdVersion:
                 type: string
-              kubeletVersion:
-                type: string
+              firstNodeToBeUpgraded:
+                description: FirstNodeToBeUpgraded signifies that the Node is the
+                  first node to be upgraded. This flag is only valid for control plane
+                  nodes and ignored for worker nodes.
+                type: boolean
               kubernetesVersion:
                 type: string
               machine:
+                description: Machine is a reference to the CAPI Machine that needs
+                  to be upgraded.
                 properties:
-                  kind:
+                  apiVersion:
+                    description: API version of the referent.
                     type: string
-                  name:
+                  fieldPath:
+                    description: 'If referring to a piece of an object instead of
+                      an entire object, this string should contain a valid JSON/Go
+                      field access statement, such as desiredState.manifest.containers[2].
+                      For example, if the object reference is to a container within
+                      a pod, this would take on a value like: "spec.containers{name}"
+                      (where "name" refers to the name of the container that triggered
+                      the event) or if no container name is specified "spec.containers[2]"
+                      (container with index 2 in this pod). This syntax is chosen
+                      only to have some well-defined way of referencing a part of
+                      an object. TODO: this design is not final and this field is
+                      subject to change in the future.'
                     type: string
-                type: object
-              node:
-                properties:
                   kind:
+                    description: 'Kind of the referent. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
                     type: string
                   name:
+                    description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names'
+                    type: string
+                  namespace:
+                    description: 'Namespace of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/namespaces/'
+                    type: string
+                  resourceVersion:
+                    description: 'Specific resourceVersion to which this reference
+                      is made, if any. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency'
+                    type: string
+                  uid:
+                    description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids'
                     type: string
                 type: object
             required:
-            - kubeletVersion
             - kubernetesVersion
             - machine
-            - node
             type: object
           status:
             description: NodeUpgradeStatus defines the observed state of NodeUpgrade.
             properties:
               completed:
                 type: boolean
               conditions:
-                description: Conditions provide observations of the operational state
-                  of a Cluster API resource.
                 items:
                   description: Condition defines an observation of a Cluster API resource
                     operational state.
@@ -4968,11 +4990,11 @@ spec:
                   - type
                   type: object
                 type: array
-              phase:
-                type: string
-            required:
-            - completed
-            - phase
+              observedGeneration:
+                description: ObservedGeneration is the latest generation observed
+                  by the controller.
+                format: int64
+                type: integer
             type: object
         type: object
     served: true
@@ -6659,6 +6681,16 @@ rules:
   - nodes
   verbs:
   - list
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:
@@ -6888,6 +6920,16 @@ rules:
   - list
   - patch
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - machines
+  verbs:
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - clusterctl.cluster.x-k8s.io
   resources:

config/rbac/role.yaml

@@ -27,6 +27,16 @@ rules:
   - nodes
   verbs:
   - list
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:
@@ -256,6 +266,16 @@ rules:
   - list
   - patch
   - watch
+- apiGroups:
+  - cluster.x-k8s.io
+  resources:
+  - machines
+  verbs:
+  - get
+  - list
+  - patch
+  - update
+  - watch
 - apiGroups:
   - clusterctl.cluster.x-k8s.io
   resources:

controllers/factory.go

@@ -590,13 +590,15 @@ func (f *Factory) WithMachineDeploymentUpgradeReconciler() *Factory {
 
 // WithNodeUpgradeReconciler builds the WithNodeUpgrade reconciler.
 func (f *Factory) WithNodeUpgradeReconciler() *Factory {
+	f.withTracker()
 	f.buildSteps = append(f.buildSteps, func(ctx context.Context) error {
 		if f.reconcilers.NodeUpgradeReconciler != nil {
 			return nil
 		}
 
 		f.reconcilers.NodeUpgradeReconciler = NewNodeUpgradeReconciler(
 			f.manager.GetClient(),
+			f.tracker,
 		)
 
 		return nil

controllers/factory_test.go

@@ -229,3 +229,63 @@ func TestFactoryWithNutanixDatacenterReconciler(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(reconcilers.NutanixDatacenterReconciler).NotTo(BeNil())
 }
+
+func TestFactoryWithNodeUpgradeReconciler(t *testing.T) {
+	g := NewWithT(t)
+	ctx := context.Background()
+	logger := nullLog()
+	ctrl := gomock.NewController(t)
+	manager := mocks.NewMockManager(ctrl)
+	manager.EXPECT().GetClient().AnyTimes()
+	manager.EXPECT().GetScheme().AnyTimes()
+
+	f := controllers.NewFactory(logger, manager).
+		WithNodeUpgradeReconciler()
+
+	// testing idempotence
+	f.WithNodeUpgradeReconciler()
+
+	reconcilers, err := f.Build(ctx)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(reconcilers.NodeUpgradeReconciler).NotTo(BeNil())
+}
+
+func TestFactoryWithControlPlaneUpgradeReconciler(t *testing.T) {
+	g := NewWithT(t)
+	ctx := context.Background()
+	logger := nullLog()
+	ctrl := gomock.NewController(t)
+	manager := mocks.NewMockManager(ctrl)
+	manager.EXPECT().GetClient().AnyTimes()
+	manager.EXPECT().GetScheme().AnyTimes()
+
+	f := controllers.NewFactory(logger, manager).
+		WithControlPlaneUpgradeReconciler()
+
+	// testing idempotence
+	f.WithControlPlaneUpgradeReconciler()
+
+	reconcilers, err := f.Build(ctx)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(reconcilers.ControlPlaneUpgradeReconciler).NotTo(BeNil())
+}
+
+func TestFactoryWithMachineDeploymentUpgradeReconciler(t *testing.T) {
+	g := NewWithT(t)
+	ctx := context.Background()
+	logger := nullLog()
+	ctrl := gomock.NewController(t)
+	manager := mocks.NewMockManager(ctrl)
+	manager.EXPECT().GetClient().AnyTimes()
+	manager.EXPECT().GetScheme().AnyTimes()
+
+	f := controllers.NewFactory(logger, manager).
+		WithMachineDeploymentUpgradeReconciler()
+
+	// testing idempotence
+	f.WithMachineDeploymentUpgradeReconciler()
+
+	reconcilers, err := f.Build(ctx)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(reconcilers.MachineDeploymentUpgradeReconciler).NotTo(BeNil())
+}