operator: implement roundtripping conversion of v1alpha3 by chrisseto · Pull Request #892 · redpanda-data/redpanda-operator

chrisseto · 2025-06-06T21:49:58Z

This commit adds the initial set of conversion routines to support roundtripping all valid configurations of the v1alpha3 CR through the v1alpha2 CR.

To aid in the development and testing of the conversion routines, the exhaustruct linter has been enabled on files ending in _conversion.go. It requires all fields to be explicitly declared which will prevent future additions to either CR from causing regressions.

It should be noted that v1alpha2 can NOT be roundtripped. Therefore the v1alpha3 CRD can not be safely enabled.

Future work will:

Improve user facing documentation of all fields
Support roundtripping v1alpha2 through v1alpha3
(Potentially) refactor conversion to make use of the conversion-gen tool.

github-actions · 2025-06-16T02:12:42Z

This PR is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 5 days.

RafalKorepta

LGTM

operator/api/redpanda/v1alpha2/redpanda_clusterspec_types.go

RafalKorepta · 2025-06-16T14:15:42Z

operator/api/redpanda/v1alpha3/nodepool_types.go

+	// TODO does rpk redpanda tune read from redpanda.yaml?
+	// Arguments to be passed to rpk redpanda tune
+	// https://docs.redpanda.com/current/reference/rpk/rpk-redpanda/rpk-redpanda-tune/
+	// Tuning                    []string               `json:"tuning"`


@r-vasquez As far as I remember the rpk redpanda start command will not do anything with tuning configuration inside redpanda.yaml.

@chrisseto Those tuning option could take effect when the init container named tuning runs.
Reference:

redpanda-operator/charts/redpanda/statefulset.go

Lines 358 to 389 in 3153bc8

func statefulSetInitContainerTuning(dot *helmette.Dot) *corev1.Container {

values := helmette.Unwrap[Values](dot.Values)

if !values.Tuning.TuneAIOEvents {

return nil

}

return &corev1.Container{

Name: "tuning",

Image: fmt.Sprintf("%s:%s", values.Image.Repository, Tag(dot)),

Command: []string{

`/bin/bash`,

`-c`,

`rpk redpanda tune all`,

},

SecurityContext: &corev1.SecurityContext{

Capabilities: &corev1.Capabilities{

Add: []corev1.Capability{`SYS_RESOURCE`},

},

Privileged: ptr.To(true),

RunAsUser: ptr.To(int64(0)),

RunAsGroup: ptr.To(int64(0)),

},

VolumeMounts: append(

CommonMounts(dot),

corev1.VolumeMount{

Name: "base-config",

MountPath: "/etc/redpanda",

},

),

}

}

RafalKorepta · 2025-06-16T14:18:26Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+				return nil, false
+			}
+
+			// TODO Handle all cases here


Those Handle all cases here means follow up PR? Or when we discover problems during cloud migration? Or something completely different?

RafalKorepta · 2025-06-16T14:19:51Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+				Memory:   nil,
+			},
+
+			// TODO Audit this list.


By audit you mean handle them in next PR or leave them as nil?

RafalKorepta · 2025-06-16T14:20:40Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+			AdditionalRedpandaCmdFlags:    nil,
+			AdditionalSelectorsLabels:     nil,
+			Annotations:                   nil,
+			Budget:                        nil,
+			ExtraVolumeMounts:             nil,
+			ExtraVolumes:                  nil,
+			InitContainerImage:            nil,
+			LivenessProbe:                 nil,
+			NodeSelector:                  nil,
+			PodAffinity:                   nil,
+			PodAntiAffinity:               nil,
+			PriorityClassName:             nil,
+			ReadinessProbe:                nil,
+			SecurityContext:               nil,
+			SideCars:                      nil,
+			SkipChown:                     nil,
+			StartupProbe:                  nil,
+			TerminationGracePeriodSeconds: nil,
+			Tolerations:                   nil,
+			TopologySpreadConstraints:     nil,
+			UpdateStrategy:                nil,


Should those fields have similar TODO Audit this list..

operator/api/redpanda/v1alpha3/redpanda_conversion_test.go

operator/api/redpanda/v1alpha3/redpanda_types.go

pkg/rapidutil/rapidutil.go

andrewstucki

I'm fine with this as a first pass since nothing is using it, but lots of questions before I hit approve. Also, WDYT about a rapid test that roundtrips v1alpha2 --> 3 --> 2 but with deprecated/unsupported fields ignored (as mentioned in the PR description, I'm fine doing this secondarily)? I'm assuming that was what the start of the commented test case is?

.golangci.yml

operator/api/redpanda/v1alpha2/redpanda_clusterspec_types.go

andrewstucki · 2025-06-16T17:08:40Z

operator/api/redpanda/v1alpha3/common.go

 // - addresses: Expr(srv_address('tcp', 'admin', 'redpanda.redpanda.cluster.svc.cluster.local'))
 type Expr string

+type ObjectMeta struct {


Is there a reason not to just use or alias metav1.ObjectMeta? I'm assuming b/c we don't want to pull in name/namespace fields?

andrewstucki · 2025-06-16T17:46:57Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+			Enabled:           ptr.To(false),
+			Cert:              nil,
+			RequireClientAuth: nil,
+			SecretRef:         nil,


Looks like this is unused. Was this slurped into Cert at some point but never dropped?

andrewstucki · 2025-06-16T17:47:25Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+			SecretRef: &redpandav1alpha2.SecretRef{
+				Name: ptr.To(tls.Secrets.SecretRef.Name),
+			},
+			ApplyInternalDNSNames: nil,


deprecated?

andrewstucki · 2025-06-16T17:47:36Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+			ApplyInternalDNSNames: nil,
+			CAEnabled:             nil,
+			ClientSecretRef:       clientRef,
+			Duration:              nil,


deprecated?

andrewstucki · 2025-06-16T17:48:56Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+				Name: ptr.To(tls.Secrets.SecretRef.Name),
+			},
+			ApplyInternalDNSNames: nil,
+			CAEnabled:             nil,


deprecated? all of the above appear unused. I just worry if we haven't explicitly deprecated these fields, we'll lose behavior roundtripping.

andrewstucki · 2025-06-16T17:49:53Z

operator/api/redpanda/v1alpha3/redpanda_conversion.go

+	}
+
+	var source CertificateSource
+	if cert.IssuerRef != nil {


again, mutually exclusive?

This commit adds the initial set of conversion routines to support roundtripping all valid configurations of the v1alpha3 CR through the v1alpha2 CR. To aid in the development and testing of the conversion routines, the `exhaustruct` linter has been enabled on files ending in `_conversion.go`. It requires all fields to be explicitly declared which will prevent future additions to either CR from causing regressions. It should be noted that v1alpha2 can NOT be roundtripped. Therefore the v1alpha3 CRD can not be _safely_ enabled. Future work will: - Improve user facing documentation of all fields - Support roundtripping v1alpha2 through v1alpha3 - (Potentially) refactor conversion to make use of the conversion-gen tool.

chrisseto · 2025-06-16T19:51:11Z

TFTRs! I realize this PR is a lot more half baked than I was remembering. I'm going to take a quick stab at the conversion-gen approach and adding both directions of round trips to see if I can make this feel less janky. There's so many TODOs that I will forget the context of if I merge it as is :/

github-actions · 2025-06-22T02:12:46Z

This PR is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions · 2025-07-03T02:12:16Z

This PR is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions · 2025-07-08T02:12:30Z

This PR was closed because it has been stalled for 5 days with no activity.

chrisseto requested review from RafalKorepta, andrewstucki and gene-redpanda as code owners June 6, 2025 21:50

chrisseto force-pushed the chris/p/k8s-600-004-conversion branch from 68351e9 to 3f2854a Compare June 9, 2025 18:58

chrisseto added the no-changelog label Jun 9, 2025

chrisseto force-pushed the chris/p/k8s-600-004-conversion branch from 3f2854a to 7d306eb Compare June 10, 2025 14:25

github-actions bot added the stale label Jun 16, 2025

chrisseto removed the stale label Jun 16, 2025

chrisseto force-pushed the chris/p/k8s-600-004-conversion branch from 7d306eb to b329522 Compare June 16, 2025 14:02

RafalKorepta approved these changes Jun 16, 2025

View reviewed changes

andrewstucki reviewed Jun 16, 2025

View reviewed changes

chrisseto force-pushed the chris/p/k8s-600-004-conversion branch from b329522 to 807d5c3 Compare June 16, 2025 19:49

github-actions bot added the stale label Jun 22, 2025

chrisseto removed the stale label Jun 27, 2025

github-actions bot added the stale label Jul 3, 2025

github-actions bot closed this Jul 8, 2025

	func statefulSetInitContainerTuning(dot helmette.Dot) corev1.Container {
	values := helmette.Unwrap[Values](dot.Values)

	if !values.Tuning.TuneAIOEvents {
	return nil
	}

	return &corev1.Container{
	Name: "tuning",
	Image: fmt.Sprintf("%s:%s", values.Image.Repository, Tag(dot)),
	Command: []string{
	`/bin/bash`,
	`-c`,
	`rpk redpanda tune all`,
	},
	SecurityContext: &corev1.SecurityContext{
	Capabilities: &corev1.Capabilities{
	Add: []corev1.Capability{`SYS_RESOURCE`},
	},
	Privileged: ptr.To(true),
	RunAsUser: ptr.To(int64(0)),
	RunAsGroup: ptr.To(int64(0)),
	},
	VolumeMounts: append(
	CommonMounts(dot),
	corev1.VolumeMount{
	Name: "base-config",
	MountPath: "/etc/redpanda",
	},
	),
	}
	}

Conversation

chrisseto commented Jun 6, 2025

Uh oh!

github-actions bot commented Jun 16, 2025

Uh oh!

RafalKorepta left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

andrewstucki left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

chrisseto commented Jun 16, 2025

Uh oh!

github-actions bot commented Jun 22, 2025

Uh oh!

github-actions bot commented Jul 3, 2025

Uh oh!

github-actions bot commented Jul 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrewstucki left a comment •

edited

Loading