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

Set explicitly entry point for configurator #368

Merged
merged 1 commit into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions .changes/unreleased/operator-Changed-20250109-091351.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
project: operator
kind: Changed
body: |
For any user that is mirroring configurator image and changes entrypoint or wraps configurator with additional
script the following constraint need to be meet:
* use the alternative configuration parameter by specifying `additionalCmdFlags` in Redpanda operator helm chart
* to change the container repository set `--configurator-base-image=my.repo.com/configurator`
* to change the container tag set `--configurator-tag=XYZ`
* image needs to supports the entrypoint `redpanda-operator configure` as it is the default one
time: 2025-01-09T09:13:51.668314+01:00
2 changes: 1 addition & 1 deletion .goreleaser.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ builds:
- amd64
- arm64
ldflags:
- -X "github.com/redpanda-data/redpanda-operator/operator/cmd/version.version={{.Version}}"
- -X "github.com/redpanda-data/redpanda-operator/operator/cmd/version.Version={{.Version}}"
- -X "github.com/redpanda-data/redpanda-operator/operator/cmd/version.commit={{.Commit}}"
- -X "github.com/redpanda-data/redpanda-operator/operator/cmd/version.buildDate={{.Date}}"

Expand Down
11 changes: 0 additions & 11 deletions charts/operator/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,6 @@ func containerImage(dot *helmette.Dot) string {
return fmt.Sprintf("%s:%s", values.Image.Repository, tag)
}

func configuratorTag(dot *helmette.Dot) string {
values := helmette.Unwrap[Values](dot.Values)

if !helmette.Empty(values.Configurator.Tag) {
return *values.Configurator.Tag
}
return dot.Chart.AppVersion
}

func isWebhookEnabled(dot *helmette.Dot) bool {
values := helmette.Unwrap[Values](dot.Values)

Expand Down Expand Up @@ -366,8 +357,6 @@ func operatorArguments(dot *helmette.Dot) []string {
"--health-probe-bind-address=:8081",
"--metrics-bind-address=127.0.0.1:8080",
"--leader-elect",
fmt.Sprintf("--configurator-tag=%s", configuratorTag(dot)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the corresponding values be removed or marked as deprecated as they're now a no-op?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can mark it as deprecated. My intent was to remove argument form deployment arguments to not confuse end user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After more thought both option should stay as is. Any user could potentially want to overwrite the "configurator" container registry and it's tag. It shouldn't be encourage, but some K8S cluster can be cut from the internet. Internal container registry would be then used and user is free to specify helm chart additionalCmdFlags to pass configurator-tag and configurator-base-image values

Copy link
Contributor

Choose a reason for hiding this comment

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

but some K8S cluster can be cut from the internet

Great call out!

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean you'll be reverting the change this comment thread is on?

Copy link
Contributor Author

@RafalKorepta RafalKorepta Jan 8, 2025

Choose a reason for hiding this comment

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

Nope I want to leave this unspecified by default and allow for overwrite with additionalCmdFlags if particular user is air gapped.

fmt.Sprintf("--configurator-base-image=%s", values.Configurator.Repository),
fmt.Sprintf("--webhook-enabled=%t", isWebhookEnabled(dot)),
}

Expand Down
18 changes: 1 addition & 17 deletions charts/operator/templates/_deployment.go.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -101,22 +101,6 @@
{{- end -}}
{{- end -}}

{{- define "operator.configuratorTag" -}}
{{- $dot := (index .a 0) -}}
{{- range $_ := (list 1) -}}
{{- $_is_returning := false -}}
{{- $values := $dot.Values.AsMap -}}
{{- if (not (empty $values.configurator.tag)) -}}
{{- $_is_returning = true -}}
{{- (dict "r" $values.configurator.tag) | toJson -}}
{{- break -}}
{{- end -}}
{{- $_is_returning = true -}}
{{- (dict "r" $dot.Chart.AppVersion) | toJson -}}
{{- break -}}
{{- end -}}
{{- end -}}

{{- define "operator.isWebhookEnabled" -}}
{{- $dot := (index .a 0) -}}
{{- range $_ := (list 1) -}}
Expand Down Expand Up @@ -194,7 +178,7 @@
{{- range $_ := (list 1) -}}
{{- $_is_returning := false -}}
{{- $values := $dot.Values.AsMap -}}
{{- $args := (list "--health-probe-bind-address=:8081" "--metrics-bind-address=127.0.0.1:8080" "--leader-elect" (printf "--configurator-tag=%s" (get (fromJson (include "operator.configuratorTag" (dict "a" (list $dot) ))) "r")) (printf "--configurator-base-image=%s" $values.configurator.repository) (printf "--webhook-enabled=%t" (get (fromJson (include "operator.isWebhookEnabled" (dict "a" (list $dot) ))) "r"))) -}}
{{- $args := (list "--health-probe-bind-address=:8081" "--metrics-bind-address=127.0.0.1:8080" "--leader-elect" (printf "--webhook-enabled=%t" (get (fromJson (include "operator.isWebhookEnabled" (dict "a" (list $dot) ))) "r"))) -}}
{{- if (eq $values.scope "Namespace") -}}
{{- $args = (concat (default (list ) $args) (list (printf "--namespace=%s" $dot.Release.Namespace) (printf "--log-level=%s" $values.logLevel))) -}}
{{- end -}}
Expand Down
Loading