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

Feature/3190 expand k8s nodes capabilities with storage from rook #3217

Conversation

norbix
Copy link

@norbix norbix commented Jul 13, 2022

WIP state.

@@ -6,3 +6,4 @@ specification:
version: 1.22.4
cni_version: 0.8.7
node_labels: "node-type=epiphany"
enable_controller_attach_detach: false
Copy link
Contributor

Choose a reason for hiding this comment

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

As proposed by @to-bar, we can move this argument to kubernetes-master configuration.
Name of the file we want to move it to: schema/common/defaults/configuration/kubernetes-master.yml

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -10,3 +10,4 @@ nodeRegistration:
kubeletExtraArgs:
enable-controller-attach-detach: "false"
node-labels: {{ specification.node_labels }}
enable_controller_attach_detach: "{{ specification.enable_controller_attach_detach }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

As commented #3190 (comment), we don't want to setup this flag in kubeadm-join-node, so this argument shouldn't be present in this file.
Only location to configure this argument should be ansible/playbooks/roles/kubernetes_master/templates/kubeadm-config.yml.j2

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -1,4 +1,9 @@
---
- name: Load default variables for k8s nodes
Copy link
Contributor

Choose a reason for hiding this comment

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

After moving parameter enable_controller_attach_detach to schema/common/defaults/configuration/kubernetes-master.yml loading default variables for k8s nodes will be obsolete.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@@ -61,3 +61,4 @@ kind: KubeletConfiguration
apiVersion: kubelet.config.k8s.io/v1beta1
cgroupDriver: systemd
rotateCertificates: true
enableControllerAttachDetach: {{ kubernetes_node_vars.specification.enable_controller_attach_detach }}
Copy link
Contributor

Choose a reason for hiding this comment

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

After moving parameter enable_controller_attach_detach to schema/common/defaults/configuration/kubernetes-master.yml it can look like that:

Suggested change
enableControllerAttachDetach: {{ kubernetes_node_vars.specification.enable_controller_attach_detach }}
enableControllerAttachDetach: {{ specification.enable_controller_attach_detach }}

Copy link
Author

Choose a reason for hiding this comment

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

Done

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@seriva
Copy link
Collaborator

seriva commented Aug 2, 2022

Closing as a new pull was made for this: #3237

@seriva seriva closed this Aug 2, 2022
@tomasz-baran tomasz-baran deleted the feature/3190-expand-k8s-nodes-capabilities-with-storage-from-rook branch August 25, 2022 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants