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

add helm to k8s master for vault and fixes [#1729] #1731

Merged

Conversation

ar3ndt
Copy link
Contributor

@ar3ndt ar3ndt commented Oct 2, 2020

No description provided.

@rpudlowski93
Copy link
Contributor

LGTM, tested

@ar3ndt ar3ndt merged commit 74c5dd6 into hitachienergy:develop Oct 2, 2020
Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

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

🤔 TOO LATE :'C

when:
- specification.vault_enabled
- k8s_as_cloud_service is not defined
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this line has no effect, we can safely assume this is defined always (it's merged with defaults), no? 🤔

Suggested change
- k8s_as_cloud_service is not defined

Copy link
Contributor

Choose a reason for hiding this comment

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

Also it could mask some future issues with upgrades... 🤔

@@ -34,9 +34,11 @@
dest: /tmp/vault_helm_chart_values.yaml
when: vault_helm_chart_values_bool

- name: Install and configure Hashicorp Vault if enabled
- name: Install and configure Hashicorp Vault if enabled and no cloud service is used
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Install and configure Hashicorp Vault if enabled and no cloud service is used
- name: Install and configure Hashicorp Vault if enabled and no cloud k8s service is used

@@ -103,3 +103,8 @@
lineinfile:
path: "/home/{{ admin_user.name }}/.bashrc"
line: source <(kubectl completion bash)

- name: Include Helm binary installation
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Include Helm binary installation
- name: Install Helm binary file

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.

4 participants