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.

Sign up for GitHub

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

🐛 Extend helm feature gates #634

Closed
wants to merge 3 commits into from
Closed

🐛 Extend helm feature gates #634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
20c0566
Add featureGates condition to bootstrap and control-plane providers
gcezaralmeida Nov 7, 2024
0a42012
Add examples for on values.yaml
gcezaralmeida Nov 7, 2024
8cc65d8
Add .Values.manager to the condition for customizing coreProvider
gcezaralmeida Nov 7, 2024
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
9 changes: 9 additions & 0 deletions hack/charts/cluster-api-operator/templates/bootstrap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ spec:
{{- if $bootstrapVersion }}
version: {{ $bootstrapVersion }}
{{- end }}
{{- if $.Values.manager }}
manager:
{{- if and $.Values.manager.featureGates $.Values.manager.featureGates.bootstrap }}
featureGates:
{{- range $key, $value := $.Values.manager.featureGates.bootstrap }}
{{ $key }}: {{ $value }}
{{- end }}
{{- end }}
{{- end }}
{{- if $.Values.configSecret.name }}
configSecret:
name: {{ $.Values.configSecret.name }}
Expand Down
10 changes: 3 additions & 7 deletions hack/charts/cluster-api-operator/templates/control-plane.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,11 @@ spec:
{{- end }}
{{- if $.Values.manager }}
manager:
{{- if hasKey $.Values.manager.featureGates $controlPlaneName }}
{{- range $key, $value := $.Values.manager.featureGates }}
{{- if eq $key $controlPlaneName }}
{{- if and $.Values.manager.featureGates $.Values.manager.featureGates.controlPlane }}
featureGates:
{{- range $k, $v := $value }}
{{ $k }}: {{ $v }}
{{- range $key, $value := $.Values.manager.featureGates.controlPlane }}
Copy link

Choose a reason for hiding this comment

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

$k, $v is a 'standard' for Helm ranges, so it can be omitted.

{{ $key }}: {{ $value }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- end }}
{{- if $.Values.configSecret.name }}
Expand Down
2 changes: 1 addition & 1 deletion hack/charts/cluster-api-operator/templates/core.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ metadata:
"helm.sh/hook": "post-install,post-upgrade"
"helm.sh/hook-weight": "2"
"argocd.argoproj.io/sync-wave": "2"
{{- if or $coreVersion $.Values.configSecret.name }}
{{- if or $coreVersion $.Values.configSecret.name $.Values.manager }}
spec:
{{- end}}
{{- if $coreVersion }}
Expand Down
12 changes: 11 additions & 1 deletion hack/charts/cluster-api-operator/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,17 @@ bootstrap: ""
controlPlane: ""
infrastructure: ""
addon: ""
manager.featureGates: {}
manager:
featureGates: {}
# e.g.
# manager:
# featureGates:
# core:
# KubeadmBootstrapFormatIgnition: true
# bootstrap:
# KubeadmBootstrapFormatIgnition: true
# controlPlane:
# KubeadmBootstrapFormatIgnition: true
Comment on lines +12 to +19
Copy link

Choose a reason for hiding this comment

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

Out of scope for this PR, but values.yaml already has core providers listed at the top level - would it make sense to lay-out all the config / params under, instead of duplicating them all over?

e.g.

core:            # <- everything for core provider in once place
  version: "v0.1.0"
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: false
  deployment:
    ...
  configSecret:
    ...
  fetchConfig:
    ...
bootstrap:       # <- everything for bootstrap provider in once place
  version: ""
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: true 
controlPlane:    # <- everything for controlPlane provider in once place
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: true

Copy link
Author

Choose a reason for hiding this comment

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

I think this approach makes sense. Instead of using key-value pairs, we could use an array. This will provide more flexibility, allowing specific configurations for each provider when multiple providers are used. Let me know what you think, I can work on that.

core:            # <- everything for core provider in once place
- name: "cluster-api"
  version: "v0.1.0"
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: false
  deployment:
    ...
  configSecret:
    ...
  fetchConfig:
    ...
bootstrap:
- name: "kubeadm"       # <- everything for bootstrap provider in once place
  version: ""
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: true 
controlPlane:
- name: "kubeadm"
  version: ""    # <- everything for controlPlane provider in once place
  manager:
    featureGates:
      KubeadmBootstrapFormatIgnition: true

Copy link
Author

Choose a reason for hiding this comment

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

@furkatgofurov7, Do you think that we can go with this PR? Or do we restructure the values.yaml like above example?

# ---
# Common configuration secret options
configSecret: {}
Expand Down
Loading