Update containerd_insecure_registries to match containerd_registries #8340

mircyb · 2021-12-25T23:35:33Z

i think this commit code is not completed works

exam registry address : a.com:5000

insecure registry must be http://a.com:5000

but this code add insecure a.com:5000 (without http://)

If there is no http, containerd accesses with https even if insecure_skip_verify = true

solution is code edit

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:
/kind feature
Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

[containerd] make containerd_insecure_registries into a dict similar to containerd_registries [⚠️ ADD NOTE `containerd_insecure_registries` needs to be updated or won't work anymore]

i think this commit code is not completed works exam registry address : a.com:5000 insecure registry must be http://a.com:5000 but this code add insecure a.com:5000 (without http://) If there is no http, containerd accesses with https even if insecure_skip_verify = true solution is code edit

k8s-ci-robot · 2021-12-25T23:35:40Z

Hi @mircyb. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

liupeng0518 · 2021-12-30T08:27:02Z

roles/container-engine/containerd/templates/config.toml.j2

@@ -56,7 +56,7 @@ oom_score = {{ containerd_oom_score }}
 {% endfor %}
 {% for addr in containerd_insecure_registries %}
        [plugins."io.containerd.grpc.v1.cri".registry.mirrors."{{ addr }}"]
-          endpoint = ["{{ ([ addr ] | flatten ) | join('","') }}"]
+          endpoint = ["http://{{ ([ addr ] | flatten ) | join('","') }}"]


what about https ?

https registry setting here and work fine

{% for registry, addr in containerd_registries.items() %}
[plugins."io.containerd.grpc.v1.cri".registry.mirrors."{{ registry }}"]
endpoint = ["{{ ([ addr ] | flatten ) | join('","') }}"]
{% endfor %}

If endpoint is no http it will always try with https

How about updating sample inventory instead by adding "http://"?
Current sample is

# containerd_insecure_registries: # - mirror.registry.io # - 172.19.16.11:5000 # containerd_registries: # "docker.io": "https://registry-1.docker.io"

As we see, the sample of containerd_registries contains "https://".
It would be consistent by adding "http://" to the sample of containerd_insecure_registries, I guess.

thank you for your advice,

i fixed code and doc for consistent

oomichi

/cc @oomichi

oomichi · 2022-01-04T22:23:37Z

roles/container-engine/containerd/templates/config.toml.j2

@@ -56,7 +56,7 @@ oom_score = {{ containerd_oom_score }}
 {% endfor %}
 {% for addr in containerd_insecure_registries %}
        [plugins."io.containerd.grpc.v1.cri".registry.mirrors."{{ addr }}"]
-          endpoint = ["{{ ([ addr ] | flatten ) | join('","') }}"]
+          endpoint = ["http://{{ ([ addr ] | flatten ) | join('","') }}"]


How about updating sample inventory instead by adding "http://"?
Current sample is

# containerd_insecure_registries: # - mirror.registry.io # - 172.19.16.11:5000 # containerd_registries: # "docker.io": "https://registry-1.docker.io"

As we see, the sample of containerd_registries contains "https://".
It would be consistent by adding "http://" to the sample of containerd_insecure_registries, I guess.

oomichi · 2022-01-05T02:03:31Z

Thanks for updating.
This changes how to specify containerd_insecure_registries for users.
Then please update Does this PR introduce a user-facing change?: on this pull request message instead of NONE.

oomichi · 2022-01-05T02:03:44Z

/ok-to-test

cristicalin · 2022-01-05T07:39:06Z

containerd Insecure registry config modify

I would reword this as:

[containerd] make containerd_insecure_registries into a dict similar to containerd_registries

This conveys more clearly the fact that a public facing configuration variable has changed format.

mircyb · 2022-01-05T07:44:56Z

containerd Insecure registry config modify

I would reword this as:

[containerd] make containerd_insecure_registries into a dict similar to containerd_registries

This conveys more clearly the fact that a public facing configuration variable has changed format.

thx

cristicalin · 2022-01-05T10:18:22Z

@mircyb 👍

/lgtm

floryut

@mircyb nice work 👍

k8s-ci-robot · 2022-01-05T10:52:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, mircyb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [floryut]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

tmurakam · 2022-02-12T12:55:00Z

I'd like to backport pull request to release-2.18 branch.
Without this, I can't access private http registry from containerd.

Backport kubernetes-sigs#8340 to 2.18-release Cherry-pick dda557e

* Update config.toml.j2 i think this commit code is not completed works exam registry address : a.com:5000 insecure registry must be http://a.com:5000 but this code add insecure a.com:5000 (without http://) If there is no http, containerd accesses with https even if insecure_skip_verify = true solution is code edit * Update config.toml.j2 * Update containerd.yml * Update containerd.yml * Update containerd.yml * Update config.toml.j2 (cherry picked from commit dda557e)

* Update config.toml.j2 i think this commit code is not completed works exam registry address : a.com:5000 insecure registry must be http://a.com:5000 but this code add insecure a.com:5000 (without http://) If there is no http, containerd accesses with https even if insecure_skip_verify = true solution is code edit * Update config.toml.j2 * Update containerd.yml * Update containerd.yml * Update containerd.yml * Update config.toml.j2 (cherry picked from commit dda557e) Co-authored-by: Choi Yongbeom <59861163+mircyb@users.noreply.github.com>

* Update config.toml.j2 i think this commit code is not completed works exam registry address : a.com:5000 insecure registry must be http://a.com:5000 but this code add insecure a.com:5000 (without http://) If there is no http, containerd accesses with https even if insecure_skip_verify = true solution is code edit * Update config.toml.j2 * Update containerd.yml * Update containerd.yml * Update containerd.yml * Update config.toml.j2

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 25, 2021

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 25, 2021

k8s-ci-robot requested review from cristicalin and EppO December 25, 2021 23:35

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 25, 2021

liupeng0518 reviewed Dec 31, 2021

View reviewed changes

oomichi reviewed Jan 4, 2022

View reviewed changes

k8s-ci-robot requested a review from oomichi January 4, 2022 22:24

mircyb added 3 commits January 5, 2022 10:47

Update config.toml.j2

c98d8a7

Update containerd.yml

69633a6

Update containerd.yml

a49cf7e

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 5, 2022

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 5, 2022

mircyb added 2 commits January 5, 2022 13:25

Update containerd.yml

bb3c2c5

Update config.toml.j2

ade18bd

mircyb closed this Jan 5, 2022

mircyb reopened this Jan 5, 2022

k8s-ci-robot assigned cristicalin Jan 5, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 5, 2022

floryut removed the kind/feature Categorizes issue or PR as related to a new feature. label Jan 5, 2022

floryut added the kind/container-managers Containers section in the release note label Jan 5, 2022

floryut approved these changes Jan 5, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 5, 2022

k8s-ci-robot merged commit dda557e into kubernetes-sigs:master Jan 5, 2022

floryut changed the title ~~Update config.toml.j2~~ Update containerd_insecure_registries to match containerd_registries Jan 5, 2022

tmurakam added a commit to tmurakam/kubespray that referenced this pull request Mar 4, 2022

Subject: [PATCH] Update config.toml.j2 (kubernetes-sigs#8340)

9450753

Backport kubernetes-sigs#8340 to 2.18-release Cherry-pick dda557e

tmurakam added a commit to tmurakam/kubespray that referenced this pull request Mar 4, 2022

Subject: [PATCH] Update config.toml.j2 (kubernetes-sigs#8340)

9ce8bc6

Backport kubernetes-sigs#8340 to 2.18-release Cherry-pick dda557e

tmurakam mentioned this pull request Mar 4, 2022

[2.18] Update containerd_insecure_registries to match containerd_registries #8602

Merged

oomichi mentioned this pull request May 28, 2022

Release Proposal v2.19 #8885

Closed

yankay mentioned this pull request Jan 29, 2023

fix: with_item to with_dict #9729

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update containerd_insecure_registries to match containerd_registries #8340

Update containerd_insecure_registries to match containerd_registries #8340

mircyb commented Dec 25, 2021 •

edited by floryut

Loading

k8s-ci-robot commented Dec 25, 2021

liupeng0518 Dec 30, 2021

mircyb Dec 31, 2021

oomichi Jan 4, 2022

mircyb Jan 5, 2022

oomichi left a comment

oomichi Jan 4, 2022

oomichi commented Jan 5, 2022

oomichi commented Jan 5, 2022

cristicalin commented Jan 5, 2022

mircyb commented Jan 5, 2022

cristicalin commented Jan 5, 2022

floryut left a comment

k8s-ci-robot commented Jan 5, 2022

tmurakam commented Feb 12, 2022

Update containerd_insecure_registries to match containerd_registries #8340

Update containerd_insecure_registries to match containerd_registries #8340

Conversation

mircyb commented Dec 25, 2021 • edited by floryut Loading

k8s-ci-robot commented Dec 25, 2021

liupeng0518 Dec 30, 2021

Choose a reason for hiding this comment

mircyb Dec 31, 2021

Choose a reason for hiding this comment

oomichi Jan 4, 2022

Choose a reason for hiding this comment

mircyb Jan 5, 2022

Choose a reason for hiding this comment

oomichi left a comment

Choose a reason for hiding this comment

oomichi Jan 4, 2022

Choose a reason for hiding this comment

oomichi commented Jan 5, 2022

oomichi commented Jan 5, 2022

cristicalin commented Jan 5, 2022

mircyb commented Jan 5, 2022

cristicalin commented Jan 5, 2022

floryut left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 5, 2022

tmurakam commented Feb 12, 2022

mircyb commented Dec 25, 2021 •

edited by floryut

Loading