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.

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

Maintenance controller #8

Merged
merged 10 commits into from
Aug 9, 2024

Conversation

adrianchiris
Copy link
Collaborator

@adrianchiris adrianchiris commented Aug 7, 2024

Refer to individual commits for context.

- Add additional helper methods
- Modify some existing functions to accept k8sclient
  and update object in api.

Signed-off-by: adrianc <[email protected]>
add additional helpers for tests to be used
in subsequent commits

Signed-off-by: adrianc <[email protected]>
@coveralls
Copy link

coveralls commented Aug 7, 2024

Pull Request Test Coverage Report for Build 10301427801

Details

  • 257 of 424 (60.61%) changed or added relevant lines in 7 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+3.8%) to 60.611%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/k8sutils/k8sutils.go 39 45 86.67%
internal/cordon/cordon.go 30 41 73.17%
internal/controller/nodemaintenancescheduler_controller.go 15 27 55.56%
internal/podcompletion/podcompletion.go 44 60 73.33%
cmd/maintenance-manager/main.go 0 29 0.0%
internal/controller/nodemaintenance_controller.go 124 217 57.14%
Files with Coverage Reduction New Missed Lines %
internal/controller/nodemaintenancescheduler_controller.go 1 83.12%
internal/controller/nodemaintenance_controller.go 3 64.34%
Totals Coverage Status
Change from base Build 10161234420: 3.8%
Covered Lines: 714
Relevant Lines: 1178

💛 - Coveralls

Copy link
Collaborator

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

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

good job! Added few minor comments

internal/cordon/cordon_suite_test.go Outdated Show resolved Hide resolved
internal/podcompletion/podcompletion_suite_test.go Outdated Show resolved Hide resolved
internal/podcompletion/podcompletion.go Show resolved Hide resolved
cmd/maintenance-manager/main.go Show resolved Hide resolved
internal/controller/nodemaintenance_controller.go Outdated Show resolved Hide resolved
internal/controller/nodemaintenance_controller_test.go Outdated Show resolved Hide resolved
cordon package handles operation needed to be perfomed
for NodeMaintenance in cordon state.

Signed-off-by: adrianc <[email protected]>
podcompletion package handles operation that need to
be performed for NodeMaintenance in WaitForPodCompletion
state.

Signed-off-by: adrianc <[email protected]>
Add WaitForCompletion field in status to list
pods NodeMaintenance is waiting to be completed.

Signed-off-by: adrianc <[email protected]>
update scheduler controller to use new APIs from k8sutils
package.

Signed-off-by: adrianc <[email protected]>
this is a partial implementation of the nodemaintenance
controller.

Additional logic which was added:

- handle scheduled state for node maintenance
- handle cordon state for node maintenance
- handle waitForPodCompletion state for node maintenance
- Handle ready state for node maintenance
- support deletion flow in each of the above step
- implement new predicate to handle enqueue of events
  on node maintenance creation, change in Ready condition and
  deletion. use it when setting up reconciler with manager.

implementation leverages cordon and podcompletion pacakges
introduced in earlier commits

Signed-off-by: adrianc <[email protected]>
- properly create NodeMaintenanceReconciler with
  all required fields
- update cach indexer to support listing pods by spec.nodeName
  to be used by podcompletion package
- update rbac yaml

Signed-off-by: adrianc <[email protected]>
Signed-off-by: adrianc <[email protected]>
@adrianchiris adrianchiris force-pushed the maintenance-controller branch from b478f03 to 507d331 Compare August 8, 2024 11:44
@ykulazhenkov ykulazhenkov self-requested a review August 9, 2024 06:02
Copy link
Collaborator

@ykulazhenkov ykulazhenkov left a comment

Choose a reason for hiding this comment

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

LGTM

@ykulazhenkov ykulazhenkov merged commit 7ccc345 into Mellanox:main Aug 9, 2024
8 checks passed
@adrianchiris adrianchiris deleted the maintenance-controller branch January 9, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants