-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 default values to roles #509
➕ Add default values to roles #509
Conversation
@timothystewart6 It would be great to receive your input on whether or not such a PR is in the roadmap for this project. |
@timothystewart6 Did you have a chance to look at this? |
Hi! Yes, this sounds great as long as we don't break how it currently works. Unfortunately CI isn't working right now but I hope to have it working soon so that the tests will run! Thanks! |
Just let me know when this is ready! |
No problem, I will change the PR status from draft to ready. |
@timothystewart6 Ready for review! |
@bdsoha it seems this is failing CI see logs: https://github.com/techno-tim/k3s-ansible/actions/runs/10134226628/job/28020331172 seems like you are mixing |
@timothystewart6 I didn't choose the variable names, just used the existing ones (which might cause regression failures for users of the playbook).
|
Well they were only in a template before now they are set as defaults and used elsewhere. I think you maybe have missed some ignores. There are also conflicts too. Although it does work locally, you might be on a different version? It does need to pass CI since it's the source of truth. |
@timothystewart6 The most recent commit is |
@bdsoha sorry! Good call! There are still conflict though however |
@timothystewart6 Done! |
Unfortunately it's not passing CI |
Proposed Changes
all.yml
file.Checklist
site.yml
playbookreset.yml
playbook