⚠️ Promote v1beta2 condition in CAPD #12362

sivchari · 2025-06-15T13:16:36Z

What this PR does / why we need it:

Part of #11947

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Signed-off-by: sivchari <shibuuuu5@gmail.com>

k8s-ci-robot · 2025-06-15T13:16:43Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign neolit123 for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

test/infrastructure/docker/exp/api/v1beta1/conversion.go

Signed-off-by: sivchari <shibuuuu5@gmail.com>

sivchari · 2025-06-18T03:32:30Z

/cc @sbueringer @fabriziopandini

Ready for review 👍

sbueringer · 2025-06-18T04:06:52Z

/assign @fabriziopandini
Can you do a first round please?

fabriziopandini

Thanks @sivchari for this work, just a few nits from my side

fabriziopandini · 2025-06-18T09:02:52Z

test/infrastructure/docker/api/v1beta2/devcluster_types.go


-	// v1beta2 groups all the fields that will be added or modified in DevCluster's status with the V1Beta2 version.
+	// deprecated groups all the status fields that are deprecated and will be removed when support for v1beta1 will be dropped.


nit In order to distinguish this from deprecated.v1beta1 we used

Suggested change

// deprecated groups all the status fields that are deprecated and will be removed when support for v1beta1 will be dropped.

// deprecated groups all the status fields that are deprecated and will be removed when all the nested field are removed..

(same for the struct + other types)

fabriziopandini · 2025-06-18T09:06:00Z

test/infrastructure/docker/api/v1beta2/devcluster_types.go

@@ -119,24 +119,35 @@ type DevClusterStatus struct {
 	// +optional
 	FailureDomains clusterv1beta1.FailureDomains `json:"failureDomains,omitempty"`

-	// conditions defines current service state of the DevCluster.
+	// conditions represents the observations of a DevCluster's current state.


Let's move conditions as a first field in status + add a sentence about know conditions, e.g.

Suggested change

// conditions represents the observations of a DevCluster's current state.

// conditions represents the observations of a DevCluster's current state.

// Known condition types are Ready, ..., Paused.

Same for other types

fabriziopandini · 2025-06-18T09:11:30Z

test/infrastructure/docker/exp/api/v1beta2/dockermachinepool_types.go

 	// InfrastructureMachineKind is the kind of the infrastructure resources behind MachinePool Machines.
 	// +optional
 	InfrastructureMachineKind string `json:"infrastructureMachineKind,omitempty"`
+
+	// conditions represents the observations of a DevCluster's current state.


Suggested change

// conditions represents the observations of a DevCluster's current state.

// conditions represents the observations of a DockerMachinePool's current state.

fabriziopandini · 2025-06-18T09:28:05Z

test/infrastructure/docker/api/v1beta1/conversion.go

+	if in.Conditions != nil {
+		out.Deprecated.V1Beta1.Conditions = in.Conditions
+	}


Suggested change

if in.Conditions != nil {

out.Deprecated.V1Beta1.Conditions = in.Conditions

}

out.Deprecated.V1Beta1.Conditions = in.Conditions

We already know conditions is not nil due to the test in L148

Same for all the other Convert_v1beta1_*Status_To_v1beta2_*Status func

fabriziopandini · 2025-06-18T09:29:17Z

test/infrastructure/docker/api/v1beta1/conversion.go

+	if in.Deprecated != nil && in.Deprecated.V1Beta1 != nil {
+		if in.Deprecated.V1Beta1.Conditions != nil {
+			out.Conditions = in.Deprecated.V1Beta1.Conditions
+		}
+	}


Suggested change

if in.Deprecated != nil && in.Deprecated.V1Beta1 != nil {

if in.Deprecated.V1Beta1.Conditions != nil {

out.Conditions = in.Deprecated.V1Beta1.Conditions

}

}

if in.Deprecated != nil && in.Deprecated.V1Beta1 != nil && in.Deprecated.V1Beta1.Conditions != nil {

out.Conditions = in.Deprecated.V1Beta1.Conditions

}

Same for all the Convert_v1beta2_*Status_To_v1beta1_*Status func

sivchari added 5 commits June 15, 2025 11:26

move v1beta2 conditions top level, v1beta1 conditions to deprecated

d1a1a0d

Signed-off-by: sivchari <shibuuuu5@gmail.com>

fix convertion between docker cluster and dev cluster

0dc20e9

Signed-off-by: sivchari <shibuuuu5@gmail.com>

use randfill

3465677

Signed-off-by: sivchari <shibuuuu5@gmail.com>

implement conversion diff for capd

1e942fa

Signed-off-by: sivchari <shibuuuu5@gmail.com>

implement conversion diff for exp capd

9ccbc14

Signed-off-by: sivchari <shibuuuu5@gmail.com>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 15, 2025

k8s-ci-robot requested a review from elmiko June 15, 2025 13:16

k8s-ci-robot added the do-not-merge/needs-area PR is missing an area label label Jun 15, 2025

k8s-ci-robot requested a review from richardcase June 15, 2025 13:16

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 15, 2025

sivchari commented Jun 15, 2025

View reviewed changes

test/infrastructure/docker/exp/api/v1beta1/conversion.go Show resolved Hide resolved

sivchari added 3 commits June 18, 2025 11:52

fix conversion test

642742e

Signed-off-by: sivchari <shibuuuu5@gmail.com>

fix: build failure

abe7b3d

Signed-off-by: sivchari <shibuuuu5@gmail.com>

re-generate config

051100d

Signed-off-by: sivchari <shibuuuu5@gmail.com>

sivchari changed the title ~~[WIP] ⚠️ Promote v1beta2 condition~~ ⚠️ Promote v1beta2 condition Jun 18, 2025

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 Jun 18, 2025

k8s-ci-robot requested review from fabriziopandini and sbueringer June 18, 2025 03:32

sbueringer added the area/provider/infrastructure-docker Issues or PRs related to the docker infrastructure provider label Jun 18, 2025

k8s-ci-robot removed the do-not-merge/needs-area PR is missing an area label label Jun 18, 2025

k8s-ci-robot assigned fabriziopandini Jun 18, 2025

fabriziopandini reviewed Jun 18, 2025

View reviewed changes

fabriziopandini changed the title ~~⚠️ Promote v1beta2 condition~~ ⚠️ Promote v1beta2 condition in CAPD Jun 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

⚠️ Promote v1beta2 condition in CAPD #12362

⚠️ Promote v1beta2 condition in CAPD #12362

sivchari commented Jun 15, 2025

Uh oh!

k8s-ci-robot commented Jun 15, 2025

Uh oh!

Uh oh!

sivchari commented Jun 18, 2025

Uh oh!

sbueringer commented Jun 18, 2025

Uh oh!

fabriziopandini left a comment

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

fabriziopandini Jun 18, 2025

Uh oh!

Uh oh!


		// v1beta2 groups all the fields that will be added or modified in DevCluster's status with the V1Beta2 version.
		// deprecated groups all the status fields that are deprecated and will be removed when support for v1beta1 will be dropped.

	// conditions represents the observations of a DevCluster's current state.
	// conditions represents the observations of a DevCluster's current state.
	// Known condition types are Ready, ..., Paused.

⚠️ Promote v1beta2 condition in CAPD #12362

Are you sure you want to change the base?

⚠️ Promote v1beta2 condition in CAPD #12362

Conversation

sivchari commented Jun 15, 2025

Uh oh!

k8s-ci-robot commented Jun 15, 2025

Uh oh!

Uh oh!

sivchari commented Jun 18, 2025

Uh oh!

sbueringer commented Jun 18, 2025

Uh oh!

fabriziopandini left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!