use giantswarm/cluster to render control plane resources #263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
glitchcrab
commented
Aug 9, 2024
Comment on lines
-212
to
-218
|
|
||
| {{- define "auditLogFiles" -}} | ||
| - path: /etc/kubernetes/policies/audit-policy.yaml | ||
| permissions: "0600" | ||
| encoding: base64 | ||
| content: {{ $.Files.Get "files/etc/kubernetes/policies/audit-policy.yaml" | b64enc }} | ||
| {{- end -}} |
There was a problem hiding this comment.
this is handled by the shared chart now
glitchcrab
commented
Aug 9, 2024
| @@ -1,3 +1,4 @@ | |||
| # DEPRECATED - remove once CP and workers are rendered with cluster chart | |||
There was a problem hiding this comment.
I've added these deprecation markers to make it easier to find things to be deleted at a later date - all deprecated functions and files will be handled by the cluster chart once it is fully integrated.
|
☝️ e12e8f1 is worth viewing in isolation as it explains why the vspheremachinetemplate resources have been reworked quite heavily (see the full commit message). |
glitchcrab
force-pushed
the
cluster-chart/render-controlplane
branch
from
e12e8f1
to
a0aa7ee
Compare
glitchcrab
force-pushed
the
cluster-chart/render-controlplane
branch
from
a0aa7ee
to
0c2cefc
Compare
glitchcrab
commented
Aug 16, 2024
Comment on lines
+1
to
+13
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| {{/* | ||
| You MUST bump the name suffix here and in `values.schema.json` every time one of these files | ||
| changes its content. Automatically appending a hash of the content here doesn't work | ||
| since we'd need to edit `values.schema.json` as well, but that file is created by humans. | ||
| */}} | ||
| name: {{ include "resource.default.name" $ }}-provider-specific-files-1 | ||
| namespace: {{ $.Release.Namespace | quote }} | ||
| data: | ||
| set-hostname.sh: {{ tpl ($.Files.Get "files/opt/bin/set-hostname.sh") $ | b64enc | quote }} | ||
| type: Opaque |
There was a problem hiding this comment.
the cluster chart currently doesn't have the ability to pass arbitrary file contents to the files section of ignition, so instead we create a secret which stores the contents of the the file(s) and then the secret's contents are written out to the file on disk (see https://github.com/giantswarm/cluster-vsphere/pull/263/files#diff-91218d0fa586a5a23b86c05ebd3ed9ed502aee6ace9953024119e735d7e1557dR157-R163)
glitchcrab
commented
Aug 16, 2024
Comment on lines
174
to
243
| { | ||
| "contents": { | ||
| "install": { | ||
| "wantedBy": [ | ||
| "multi-user.target" | ||
| ] | ||
| }, | ||
| "unit": { | ||
| "description": "VMWare metadata agent" | ||
| } | ||
| }, | ||
| "dropins": [ | ||
| { | ||
| "contents": "[Unit]\nAfter=nss-lookup.target\nAfter=network-online.target\nWants=network-online.target\n[Service]\nType=oneshot\nRestart=on-failure\nRemainAfterExit=yes\nEnvironment=OUTPUT=/run/metadata/coreos\nExecStart=/usr/bin/mkdir --parent /run/metadata\nExecStart=/usr/bin/bash -cv 'echo \"COREOS_CUSTOM_HOSTNAME=$(\"$(find /usr/bin /usr/share/oem -name vmtoolsd -type f -executable 2>/dev/null | head -n 1)\" --cmd \"info-get guestinfo.metadata\" | base64 -d | awk \\'/local-hostname/ {print $2}\\' | tr -d \\'\"\\')\" >> ${OUTPUT}'\nExecStart=/usr/bin/bash -cv 'echo \"COREOS_CUSTOM_IPV4=$(\"$(find /usr/bin /usr/share/oem -name vmtoolsd -type f -executable 2>/dev/null | head -n 1)\" --cmd \"info-get guestinfo.ip\")\" >> ${OUTPUT}'", | ||
| "name": "10-coreos-metadata.conf" | ||
| } | ||
| ], | ||
| "enabled": true, | ||
| "name": "coreos-metadata.service" | ||
| }, | ||
| { | ||
| "contents": { | ||
| "install": { | ||
| "wantedBy": [ | ||
| "multi-user.target" | ||
| ] | ||
| }, | ||
| "unit": { | ||
| "description": "Set machine hostname" | ||
| } | ||
| }, | ||
| "dropins": [ | ||
| { | ||
| "contents": "[Unit]\nRequires=coreos-metadata.service\nAfter=coreos-metadata.service\n[Service]\nType=oneshot\nRemainAfterExit=yes\nEnvironmentFile=/run/metadata/coreos\nExecStart=/opt/set-hostname", | ||
| "name": "10-set-hostname.conf" | ||
| } | ||
| ], | ||
| "enabled": true, | ||
| "name": "set-hostname.service" | ||
| }, | ||
| { | ||
| "contents": { | ||
| "install": { | ||
| "wantedBy": [ | ||
| "default.target" | ||
| ] | ||
| }, | ||
| "unit": { | ||
| "description": "Disable TCP segmentation offloading" | ||
| } | ||
| }, | ||
| "dropins": [ | ||
| { | ||
| "contents": "[Unit]\nAfter=network.target\n[Service]\nType=oneshot\nRemainAfterExit=yes\nExecStart=/usr/sbin/ethtool -K ens192 tx-udp_tnl-csum-segmentation off\nExecStart=/usr/sbin/ethtool -K ens192 tx-udp_tnl-segmentation off", | ||
| "name": "10-ethtool-segmentation.conf" | ||
| } | ||
| ], | ||
| "enabled": true, | ||
| "name": "ethtool-segmentation.service" | ||
| }, | ||
| { | ||
| "dropins": [ | ||
| { | ||
| "contents": "[Unit]\n# kubeadm must run after coreos-metadata populated /run/metadata directory.\nRequires=coreos-metadata.service\nAfter=coreos-metadata.service\n[Service]\n# Make metadata environment variables available for pre-kubeadm commands.\nEnvironmentFile=/run/metadata/*", | ||
| "name": "10-flatcar.conf" | ||
| } | ||
| ], | ||
| "enabled": true, | ||
| "name": "kubeadm.service" | ||
| } |
There was a problem hiding this comment.
this section duplicates provider-specific systemd units. currently, the cluster chart doesn't have the ability to template these units out fully, so a very basic unit file is created with the options available in the schema and then any other service args are added via a drop-in file. Currently these drop-ins have to be stringified as JSON doesn't support multi-line strings.
There was a problem hiding this comment.
github shows this as outdated as the last dropin for the kubeadm service isn't required so i've removed it - the cluster chart handles this now. The rest of this section is still valid.
glitchcrab
commented
Aug 16, 2024
Comment on lines
+2
to
+13
| apiVersion: {{ include "infrastructureApiVersion" . }} | ||
| kind: VSphereMachineTemplate | ||
| metadata: | ||
| name: {{ include "resource.default.name" $ }}-control-plane-{{ include "machineTemplateSpec.hash" (dict "data" (include "controlplane-vspheremachinetemplate-spec" $) "salt" $.Values.cluster.providerIntegration.hashSalt) }} | ||
| namespace: {{ $.Release.Namespace }} | ||
| labels: | ||
| {{- include "labels.common" $ | nindent 4 }} | ||
| spec: | ||
| template: | ||
| spec: | ||
| {{- include "controlplane-vspheremachinetemplate-spec" $ | nindent 6 -}} | ||
|
|
There was a problem hiding this comment.
As the cluster chart uses different methods to create the name of the machine resources, we need to align methods. Previously, this template used a function which could range over both CP and nodepool blocks, however whilst the cluster chart is only partly integrated the two naming schemes are incompatible. So we now have an individual vspheremachinetemplate for each set of nodes.
glitchcrab
commented
Aug 16, 2024
| {{/* | ||
| Generates template spec for control plane machines. | ||
| */}} | ||
| {{- define "controlplane-vspheremachinetemplate-spec" -}} |
There was a problem hiding this comment.
this is just the mtSpec function, but improved and separated out.
glitchcrab
commented
Aug 16, 2024
Comment on lines
+13
to
+27
| {{/* | ||
| Hash function based on data provided | ||
| Expects two arguments (as a `dict`) E.g. | ||
| {{ include "hash" (dict "data" . "salt" .Values.providerIntegration.hasSalt) }} | ||
| Where `data` is the data to hash and `global` is the top level scope. | ||
|
|
||
| NOTE: this function has been copied from the giantswarm/cluster chart | ||
| (see `cluster.data.hash``) to ensure that resource naming is identical. | ||
| */}} | ||
| {{- define "machineTemplateSpec.hash" -}} | ||
| {{- $data := mustToJson .data | toString }} | ||
| {{- $salt := "" }} | ||
| {{- if .salt }}{{ $salt = .salt}}{{end}} | ||
| {{- (printf "%s%s" $data $salt) | quote | sha1sum | trunc 8 }} | ||
| {{- end -}} |
There was a problem hiding this comment.
this function is duplicated from the cluster chart and used to calculate the hash suffix which is appended to the kubeadmcontrolplane resource name
glitchcrab
commented
Aug 16, 2024
Comment on lines
+1
to
+7
| #!/bin/sh | ||
| set -x | ||
| echo "${COREOS_CUSTOM_HOSTNAME}" > /etc/hostname | ||
| hostname "${COREOS_CUSTOM_HOSTNAME}" | ||
| echo "::1 ipv6-localhost ipv6-loopback" >/etc/hosts | ||
| echo "127.0.0.1 localhost" >>/etc/hosts | ||
| echo "127.0.0.1 ${COREOS_CUSTOM_HOSTNAME}" >>/etc/hosts |
There was a problem hiding this comment.
moved out of the ignition spec
1 task
glitchcrab
force-pushed
the
cluster-chart/render-controlplane
branch
from
37dbc3c
to
c4c82dd
Compare
glitchcrab
commented
Aug 19, 2024
glitchcrab
force-pushed
the
cluster-chart/render-controlplane
branch
from
260b3c6
to
1b47627
Compare
glitchcrab
commented
Aug 19, 2024
| "controlPlane": { | ||
| "apiServer": { | ||
| "extraArgs": { | ||
| "requestheader-allowed-names": "front-proxy-client" |
There was a problem hiding this comment.
we're not entirely sure why this is in the vsphere chart - it may be possible to drop it but as we're not sure i've elected to keep it for now.
glitchcrab
commented
Aug 19, 2024
| }, | ||
| "dropins": [ | ||
| { | ||
| "contents": "[Unit]\nAfter=network.target\n[Service]\nType=oneshot\nRemainAfterExit=yes\nExecStart=/usr/sbin/ethtool -K ens192 tx-udp_tnl-csum-segmentation off\nExecStart=/usr/sbin/ethtool -K ens192 tx-udp_tnl-segmentation off", |
There was a problem hiding this comment.
we should revisit this later and turn it into a script which gets called by the Service - at the moment we're hard-coding the NIC name in which will probably bite us in the future.
glitchcrab
force-pushed
the
cluster-chart/render-controlplane
branch
from
46659fe
to
21d3e15
Compare
|
There were differences in the rendered Helm template, please check! Output |
|
/run cluster-test-suites |
cluster-test-suites
📋 View full results in Tekton Dashboard Rerun trigger: Tip To only re-run the failed test suites you can provide a |
njuettner
reviewed
Aug 20, 2024
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pr: ..