Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add implementation for NodeUpgradeController #7061

Merged
merged 9 commits into from
Nov 23, 2023
Merged

Add implementation for NodeUpgradeController #7061

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4aaea0e
Add implementation for NodeUpgradeController
abhinavmpandey08 Nov 19, 2023
c3e7239
Add conditions to NodeUpgrader
abhinavmpandey08 Nov 19, 2023
9da966e
Fix linter and add unit tests
abhinavmpandey08 Nov 20, 2023
1a24c09
improve codecov
abhinavmpandey08 Nov 20, 2023
0848350
cleanup spec objects
abhinavmpandey08 Nov 20, 2023
b00ebfb
Add some TODOs for cleanup
abhinavmpandey08 Nov 21, 2023
253c6e6
fix linter
abhinavmpandey08 Nov 21, 2023
874a81e
Add more coverage
abhinavmpandey08 Nov 21, 2023
b691d5e
resolve comments
abhinavmpandey08 Nov 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 38 additions & 16 deletions config/crd/bases/anywhere.eks.amazonaws.com_nodeupgrades.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
74 changes: 58 additions & 16 deletions config/manifest/eksa-components.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -6659,6 +6681,16 @@ rules:
- nodes
verbs:
- list
- apiGroups:
- ""
resources:
- pods
verbs:
- create
- delete
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 20 additions & 0 deletions config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ rules:
- nodes
verbs:
- list
- apiGroups:
- ""
resources:
- pods
abhinavmpandey08 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to move this to a role instead of cluster role
this gives way too many privileges to the controller

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is needed anymore since the controller is building a remote client from the kubeconfig stored on the cluster which has admin on workload clusters. I'll do some testing and remove it

verbs:
- create
- delete
- get
- list
- watch
- apiGroups:
- ""
resources:
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions controllers/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions controllers/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
51 changes: 51 additions & 0 deletions controllers/mocks/nodeupgrade_controller.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading