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

Fixes #4008 by retrieving environment variables prefixed with "CONTAINERD_" #4009

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

nicolasbrechet
Copy link
Contributor

Proposed Changes

This PR simply modifies the install script to retrieve the environment variables that are prefixed with "CONTAINERD_",thus allowing to configure containerd at installation

Types of Changes

Bugfix

Verification

Install K3s master node with this change and variables prefixed with "CONTAINERD_"
On the master node, check the content of the /etc/systemd/system/k3s.service.env

Linked Issues

#4008

User-Facing Change

NONE

Further Comments

@nicolasbrechet nicolasbrechet requested a review from a team as a code owner September 14, 2021 14:51
Copy link
Contributor

@rancher-max rancher-max left a comment

Choose a reason for hiding this comment

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

LGTM. I confirmed this works using the below. Note this didn't pull because I didn't have my proxy setup, but it is showing the expected values and using the environment variables correctly:

curl -fL https://raw.githubusercontent.com/nicolasbrechet/k3s/patch-1/install.sh | INSTALL_K3S_EXEC="--write-kubeconfig-mode 644" CONTAINERD_HTTP_PROXY="http://this.is.my.fake.proxy/" CONTAINERD_HTTPS_PROXY="https://this.is.my.fake.proxy/" INSTALL_K3S_COMMIT=699ea165231cf6d665425c50ff5c75e1831f4afb sh -

...
$ sudo cat /etc/systemd/system/k3s.service.env
CONTAINERD_HTTP_PROXY=http://this.is.my.fake.proxy/
CONTAINERD_HTTPS_PROXY=https://this.is.my.fake.proxy/

$ k describe -n kube-system pod/coredns-85cb69466-mpk6s
...
Events:
  Type     Reason                  Age                From               Message
  ----     ------                  ----               ----               -------
  Normal   Scheduled               42s                default-scheduler  Successfully assigned kube-system/coredns-85cb69466-mpk6s to ip-172-31-26-123
  Warning  FailedCreatePodSandBox  14s (x3 over 42s)  kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to get sandbox image "rancher/mirrored-pause:3.1": failed to pull image "rancher/mirrored-pause:3.1": failed to pull and unpack image "docker.io/rancher/mirrored-pause:3.1": failed to resolve reference "docker.io/rancher/mirrored-pause:3.1": failed to do request: Head "https://registry-1.docker.io/v2/rancher/mirrored-pause/manifests/3.1": proxyconnect tcp: dial tcp: lookup this.is.my.fake.proxy: no such host

@cwayne18
Copy link
Collaborator

@nicolasbrechet thank you for this and welcome to contributing to k3s! I just have one ask, can you please follow the instructions here to make sure your commits are signed off so that the DCO check can pass? https://github.com/k3s-io/k3s/pull/4009/checks?check_run_id=3600207069

Thanks again, we really appreciate the PR!

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021

Codecov Report

Merging #4009 (57ad2f6) into master (699ea16) will decrease coverage by 0.44%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4009      +/-   ##
==========================================
- Coverage   11.61%   11.16%   -0.45%     
==========================================
  Files         131      132       +1     
  Lines        8860     8928      +68     
==========================================
- Hits         1029      997      -32     
- Misses       7606     7715     +109     
+ Partials      225      216       -9     
Flag Coverage Δ
inttests ?
unittests 11.16% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tests/util/cmd.go 0.00% <0.00%> (-42.03%) ⬇️
pkg/flock/flock_unix.go 0.00% <0.00%> (-20.00%) ⬇️
pkg/etcd/etcd.go 16.26% <0.00%> (-0.26%) ⬇️
pkg/server/server.go 0.00% <0.00%> (ø)
pkg/daemons/config/types.go 50.00% <0.00%> (ø)
pkg/daemons/control/server.go 0.00% <0.00%> (ø)
pkg/etcd/controller.go
pkg/etcd/member_controller.go 0.00% <0.00%> (ø)
pkg/etcd/metadata_controller.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 699ea16...57ad2f6. Read the comment docs.

@brandond
Copy link
Contributor

brandond commented Sep 14, 2021

This is ready to go once the DCO is taken care of.

Signed-off-by: Nicolas Brechet <nicolas.brechet@swisscom.com>
@nicolasbrechet
Copy link
Contributor Author

Sorry about that! DCO is taken care of, now.

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.

None yet

5 participants