-
Notifications
You must be signed in to change notification settings - Fork 45
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
set NO_PROXY env var for containerd #2071
set NO_PROXY env var for containerd #2071
Conversation
Hello alexandre-allard-scality,My role is to assist you with the merge of this Status report is not available. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
bb82b0b
to
7081d24
Compare
History mismatchMerge commit #bb82b0b81f942754794039ac08639127053abae8 on the integration branch It is likely due to a rebase of the branch Please use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions...
- user: root | ||
- group: root | ||
- mode: 0644 | ||
- makedirs: true | ||
- dir_mode: 0755 | ||
- context: | ||
environment: >- | ||
NO_PROXY=127.0.0.1,localhost,.svc,.default,.local,{{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the allowed syntax of NO_PROXY
, but this will probably won't be enough.
First question: what's the exact symptom? When we have a HTTP_PROXY
set up, is containerd
failing on retrieval of new container images? Are containers also failing to reach other services/cluster IPs?
Second question: if containerd
uses the proxy for container networking as well, then we'll need a lot more than this (I don't even think we could be exhaustive) - all Service DNS names can use .svc.<namespace>
, or .svc.<namespace>.cluster
, or .svc.<namespace>.cluster.local
, or even remove this whole suffix and only use the Service actual name. We naturally cannot specify all of them here, so maybe we could instead try to resolve names with CoreDNS before trying to go through the proxy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During my tests, only containerd was failing to retrieve container images.
I don't think that these environment variables are used inside running containers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, then I guess we don't need all those rules, just the registry address, no? Something like metalk8s-registry-from-config.invalid
(or its actual IP:port, not sure)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to remember from the Slack chats with TS, the processes inside containers were also using the proxy variables, then failing to connect to e.g. 10.96.0.1
or whatnot. Please validate this more in-depth...
In any case:
- Where does
.default
come from? - Where does
.local
come from? - Indeed, what about the registry 'DNS name'?
298effa
to
659c742
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a few questions that need clarifications
659c742
to
8c7461e
Compare
/reset |
Reset completeI have successfully deleted this pull request's integration branches. |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
|
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small question, looks good to me otherwise, can we open a debt ticket for adding some test behind a proxy?
@gdemonet Yup, easily doable. |
8c7461e
to
fd9e83b
Compare
/approve |
We need to disable proxy usage on metalk8s internal repositories, otherwise we can't reach them if any http(s)_proxy variable is set. Refs: #2052
Set HTTP_PROXY, HTTPS_PROXY and NO_PROXY environment variables in containerd systemd unit file, Refs: #2052
Move the documentation for proxies to the bootstrap configuration section and update it with the new way to setup proxies. Refs: #2052
Set the containerd configuration generation as a requisite for containerd package installation, to avoid having to restart the service just after its installation.
b7cbbc3
to
a737ace
Compare
History mismatchMerge commit #229e41a419210b597e85957825b66dc7fd18a5ef on the integration branch It is likely due to a rebase of the branch Please use the The following options are set: approve |
Reset completeI have successfully deleted this pull request's integration branches. The following options are set: approve |
Integration data createdI have created the integration data for the additional destination branches.
The following branches will NOT be impacted:
You can set option
The following options are set: approve |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list:
The following options are set: approve |
/approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to go!
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue None. Goodbye alexandre-allard-scality. |
Component: salt
Context: Containerd is failing to pull images when a http(s) proxy is set system wide through environment variables in
/etc/environment
Summary: Set NO_PROXY environment variable with control, workload plane and K8s internal
networks in containerd systemd unit file, to avoid using system wide defined HTTP(S)
proxy, if any, when trying to pull resources from metalk8s registry.
Acceptance criteria: Be able to install metalk8s with a proxy configured following the doc https://metal-k8s.readthedocs.io/en/latest/quickstart/setup.html#proxies
Closes: #2052