✨ Update machine with v1beta2 status #11276

fabriziopandini · 2024-10-08T20:50:59Z

What this PR does / why we need it:
This PR adds code for updating machine with v1beta2 status.

In order to do so, reconcile bootstrap config, infrastructure and node will now always run, ensuring info from those resources are always surfaced (before only a small subset of those were run on reconcile delete, and only after drain and wait for volume detatch actually completed).

Notably, v1beta2 conditions will also provide visibility on several issues before visible only by checking logs, e.g.

BootstrapConfigReady condition will surface when the bootstrap config object is not found or accidentally deleted
InfrastructureReady condition will surface when the infrastructure machine object is not found or accidentally deleted
NodeHealthy and NodeReady conditions will surface when the K8s node corresponding to machine object is not found or accidentally deleted

Note: this PR is not setting the deleting condition (this is deferred to after node drain proposal is going to merge)

Which issue(s) this PR fixes:
Rif #11105

Area example:
/area api

internal/controllers/machine/machine_controller.go

internal/controllers/machine/machine_controller_phases.go

internal/controllers/machine/machine_controller_test.go

api/v1beta1/machine_types.go

internal/controllers/machine/machine_controller.go

internal/controllers/machine/machine_controller_noderef.go

util/conditions/v1beta2/getter.go

api/v1beta1/machine_types.go

internal/controllers/machine/machine_controller_status_test.go

api/v1beta1/machine_types.go

util/conditions/v1beta2/merge_strategies.go

internal/controllers/machine/machine_controller_status_test.go

k8s-ci-robot · 2024-10-11T17:58:27Z

@fabriziopandini: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-apidiff-main	`7278f4a`	link	false	`/test pull-cluster-api-apidiff-main`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

sbueringer · 2024-10-14T09:01:36Z

Thx! Really nice work

/lgtm

k8s-ci-robot · 2024-10-14T09:01:42Z

LGTM label has been added.

Git tree hash: 518c2c7a92f708847f831e4f84fb69629a10382c

sbueringer · 2024-10-14T09:02:06Z

/test pull-cluster-api-e2e-main

chrischdi · 2024-10-14T09:06:50Z

internal/controllers/machine/machine_controller.go

 	// Return early if the object or Cluster is paused.
 	if annotations.IsPaused(cluster, m) {
 		log.Info("Reconciliation is paused for this object")
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, setPausedCondition(ctx, r.Client, s)


Note: this does not work together with the above predicate because a paused object only eventually enters this code here (e.g. only when paused is set on the cluster object).

Ok to fix in a follow-up though.

chrischdi · 2024-10-14T09:29:36Z

/approve

/hold

For your convenience. We can fix the paused condition in

✨ Update machineset with v1beta2 status test #11278

k8s-ci-robot · 2024-10-14T09:29:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [chrischdi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sbueringer · 2024-10-14T09:58:52Z

Sounds good!

/hold cancel

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/api Issues or PRs related to the APIs labels Oct 8, 2024

k8s-ci-robot requested review from killianmuldoon and vincepri October 8, 2024 20:51

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 8, 2024

fabriziopandini mentioned this pull request Oct 8, 2024

Tracking issue for Improving status in CAPI resources #11105

Open

97 tasks

sbueringer reviewed Oct 9, 2024

View reviewed changes

fabriziopandini force-pushed the update-v1beta2-status-machine-controller branch 2 times, most recently from a82e73b to f3e1d84 Compare October 10, 2024 17:08

sbueringer reviewed Oct 10, 2024

View reviewed changes

sbueringer reviewed Oct 11, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2024

fabriziopandini changed the title ~~[WIP] ✨ Update machine with v1beta2 status~~ ✨ Update machine with v1beta2 status Oct 11, 2024

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 11, 2024

fabriziopandini added 7 commits October 11, 2024 17:52

Ensure reconcile external references and reconcile node are always run

9ee55a8

update status with v1beta2 conditions

4bd602b

Address feedback

250bc17

Complete update status

d4648de

Address comments

8fd2725

Fix MS tests + make mirror Node Ready more robust

d2760b4

More comments

82ee71d

fabriziopandini force-pushed the update-v1beta2-status-machine-controller branch from f0ac8d1 to 82ee71d Compare October 11, 2024 16:06

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2024

sbueringer mentioned this pull request Oct 11, 2024

🌱 v1beta2 conditions: add function for setting the Paused condition #11284

Merged

sbueringer reviewed Oct 11, 2024

View reviewed changes

More comments

7278f4a

k8s-ci-robot assigned sbueringer Oct 14, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 14, 2024

chrischdi reviewed Oct 14, 2024

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 14, 2024

k8s-ci-robot merged commit 74d34e3 into kubernetes-sigs:main Oct 14, 2024
19 of 20 checks passed

k8s-ci-robot added this to the v1.9 milestone Oct 14, 2024

fabriziopandini deleted the update-v1beta2-status-machine-controller branch November 13, 2024 14:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Update machine with v1beta2 status #11276

✨ Update machine with v1beta2 status #11276

fabriziopandini commented Oct 8, 2024

k8s-ci-robot commented Oct 11, 2024

sbueringer commented Oct 14, 2024

k8s-ci-robot commented Oct 14, 2024

sbueringer commented Oct 14, 2024

chrischdi Oct 14, 2024 •

edited

Loading

chrischdi commented Oct 14, 2024 •

edited

Loading

k8s-ci-robot commented Oct 14, 2024

sbueringer commented Oct 14, 2024

✨ Update machine with v1beta2 status #11276

✨ Update machine with v1beta2 status #11276

Conversation

fabriziopandini commented Oct 8, 2024

k8s-ci-robot commented Oct 11, 2024

sbueringer commented Oct 14, 2024

k8s-ci-robot commented Oct 14, 2024

sbueringer commented Oct 14, 2024

chrischdi Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

chrischdi commented Oct 14, 2024 • edited Loading

k8s-ci-robot commented Oct 14, 2024

sbueringer commented Oct 14, 2024

chrischdi Oct 14, 2024 •

edited

Loading

chrischdi commented Oct 14, 2024 •

edited

Loading