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 option for install behind http_proxy #384

Merged
merged 2 commits into from
Oct 21, 2023

Conversation

balazshasprai
Copy link
Contributor

Proposed Changes

Added the option to install from behind an http proxy.

If configured (false by default), it puts your http_proxy + https_proxy + no_proxy env vars into a k3s(-agent).service.d/http_proxy.conf file, before anything gets installed.
This way k3s can access the internet for images etc through the proxy.

Also added env vars to the playbooks, defaulted to '', so ansible can still download the binaries and manifests even if you're behind a proxy.

In the reset main.yml I thought I'd add a conditional task.
So it doesn't try to remove the http_proxy files and errors out when you don't have them enabled/installed.

(I've ran and re-ran the checks a bunch of times on my fork, not sure what's up with the hosted runners.
The majority of the time it failed with the gurumeditation state error and when it did go past that, it couldn't resolve DNS.
I've ran this both with http_proxy enabled and disabled, installed fine in both environments.)

Checklist

  • Tested locally
  • Ran site.yml playbook
  • Ran reset.yml playbook
  • Did not add any unnecessary changes
  • Ran pre-commit install at least once before committing
  • 🚀

@timothystewart6 timothystewart6 enabled auto-merge (squash) October 20, 2023 03:52
@timothystewart6
Copy link
Contributor

Thank you! Sorry about the CI woes. It's been pretty flaky lately.

@balazshasprai
Copy link
Contributor Author

Yeah no worries. I thought it might override the already existing proxy env vars on the runner/vagrant hosts, but according to this (printing out the ansible_facts), they're not set. So that shouldn't(?) be an issue?
Now I'm thinking it might not like the default(''), checks are currently running for a default({}) instead.
If that works, I'll tidy up the environment: in the playbooks a bit and add a new commit to this

auto-merge was automatically disabled October 20, 2023 08:12

Head branch was pushed to by a user without write access

@balazshasprai
Copy link
Contributor Author

Seems like it's fine now, if vagrant decides to cooperate.
Please retry the checks, reran them on the fork, they're all nice and green

@timothystewart6 timothystewart6 enabled auto-merge (squash) October 20, 2023 23:39
@timothystewart6 timothystewart6 merged commit e880f08 into techno-tim:master Oct 21, 2023
5 checks passed
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