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

feat(k3s_env_vars): introduce k3s_install_env_vars #124

Merged
merged 1 commit into from
May 14, 2023

Conversation

FalcoSuessgott
Copy link
Contributor

@FalcoSuessgott FalcoSuessgott commented May 14, 2023

This PR introduces a variable k3s_install_env_vars which allows the user to specify a map of strings that is used to k3s installation variables (https://docs.k3s.io/reference/env-variables).

This allows me to make settings like these:

module "k3s" {
  source = "<URI>"

  k3s_version = "latest"
  
  k3s_install_env_vars = {
    INSTALL_K3S_BIN_DIR = "/usr/bin"
  }
 
 ...
}

Note:

  • This would remove the INSTALL_K3S_SELINUX_WARN variable.
  • INSTALL_K3S_VERSION can still be set via k3s_version and is checked using variable validation.

@FalcoSuessgott FalcoSuessgott changed the title feat(k3s_env_vars): introduce variable for speicfic k3s installation … feat(k3s_env_vars): introduce k3s_install_env_vars May 14, 2023
@FalcoSuessgott FalcoSuessgott marked this pull request as draft May 14, 2023 12:25
@FalcoSuessgott FalcoSuessgott marked this pull request as ready for review May 14, 2023 12:37
Copy link
Owner

@xunleii xunleii left a comment

Choose a reason for hiding this comment

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

Hi @FalcoSuessgott, thanks for your first contribution to this project.
I just have a suggestion for the validation message and would like to know if the .tfvars is intended or if it is an error.
If it is, I'd rather have the default values directly in the variable (default) and not use a tfvars inside a module.

variables.tf Outdated Show resolved Hide resolved
@FalcoSuessgott
Copy link
Contributor Author

If it is, I'd rather have the default values directly in the variable (default) and not use a tfvars inside a module.

Good catch. That was indeed just for developing purposes. I removed it now :)

@xunleii
Copy link
Owner

xunleii commented May 14, 2023

LGTM, thanks 😄

I'll try to make a release today with this change (and other ones that a forgot to release)

@xunleii xunleii merged commit d5f3266 into xunleii:master May 14, 2023
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.

2 participants