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

Multiple registries.yaml rewrites order is not preserved in containerd config.toml #3227

Open
HoustonDad opened this issue Aug 11, 2022 · 12 comments

Comments

@HoustonDad
Copy link

HoustonDad commented Aug 11, 2022

Environmental Info:
RKE2 Version: v1.21, v1.22, v1.23

Node(s) CPU architecture, OS, and Version:
CentOS 7.9.2009 x86_64

Cluster Configuration:
1 Node Server/Agent combo

Describe the bug:
When using multiple rewrites in registries.yaml, the order of those rewrites are not preserved in the containerd configuration. This leads to RKE2 pulling from the wrong places.

Steps To Reproduce:

[root@localhost ~]# curl -sfL https://get.rke2.io | INSTALL_RKE2_CHANNEL=v1.23 sh -
[root@localhost ~]# cat registries.yaml
mirrors:
  quay.io:
    endpoint:
      - "https://harbor.internal.com:5000"
    rewrite:
      "strimzi/(.*)": "registry1.dso.mil/ironbank/opensource/strimzi/$1"
      "jetstack/(.*)": "registry1.dso.mil/ironbank/jetstack/$1"
      "(.*)": "quay.io/$1"
[root@localhost ~]# mkdir -p /etc/rancher/rke2
[root@localhost ~]# cat > /etc/rancher/rke2/config.yaml <<EOF
token: usercreatedtokenstring
write-kubeconfig-mode: "0640"
tls-san:
  - rke2.houstondad.com
EOF

[root@localhost ~]# cp registries.yaml /etc/rancher/rke2/
[root@localhost ~]# systemctl enable rke2-server && systemctl start rke2-server

Expected behavior:
It's expected that the order of rewrites is honored and preserved.

Actual behavior:
The order of rewrites appears to be put in via alphabetical order.

Additional context / logs:

[root@localhost ~]# cat registries.yaml
mirrors:
  quay.io:
    endpoint:
      - "https://harbor.internal.com:5000"
    rewrite:
      "strimzi/(.*)": "registry1.dso.mil/ironbank/opensource/strimzi/$1"
      "jetstack/(.*)": "registry1.dso.mil/ironbank/jetstack/$1"
      "(.*)": "quay.io/$1"

[root@localhost ~]# cat /var/lib/rancher/rke2/agent/etc/containerd/config.toml
[plugins.opt]
  path = "/var/lib/rancher/rke2/agent/containerd"
[plugins.cri]
  stream_server_address = "127.0.0.1"
  stream_server_port = "10010"
  enable_selinux = true
  sandbox_image = "index.docker.io/rancher/pause:3.6"
[plugins.cri.containerd]
  snapshotter = "overlayfs"
  disable_snapshot_annotations = true
[plugins.cri.containerd.runtimes.runc]
  runtime_type = "io.containerd.runc.v2"
[plugins.cri.containerd.runtimes.runc.options]
        SystemdCgroup = false

[plugins.cri.registry.mirrors]
[plugins.cri.registry.mirrors."quay.io"]
  endpoint = ["https://harbor.internal.com:5000"]
  [plugins.cri.registry.mirrors."quay.io".rewrite]
    "(.*)" = "quay.io/$1"
    "jetstack/(.*)" = "registry1.dso.mil/ironbank/jetstack/$1"
    "strimzi/(.*)" = "registry1.dso.mil/ironbank/opensource/strimzi/$1"
@brandond
Copy link
Member

brandond commented Aug 15, 2022

This is kind of a deficiency in the way we implemented rewrites. We probably should have made the rewrites a list, not a map. Since it’s a map[string]string there is no inherent order to it, so they’re essentially evaluated in random order when the matches are processed. The order in the config file does not in fact matter, and there is no way to change that behavior.

The workaround is to modify the (.*) pattern so that it doesn’t match any other patterns that you might have explicit handling for.

@brandond brandond reopened this Aug 16, 2022
@brandond
Copy link
Member

Reopening as an enhancement request. This would require changes to the registries struct, as well as to the containerd patch we’re carrying. The Rancher UI would probably also need to be modified to use the new field, and anyone who wanted to take advantage of it on their own would need to migrate their config. The registries bit would look something like:

  quay.io:
    endpoint:
      - "https://harbor.internal.com:5000/"
    rewrites:
      - source: "strimzi/(.*)"
        replacement: "registry1.dso.mil/ironbank/opensource/strimzi/$1"
      - source: "jetstack/(.*)"
        replacement "registry1.dso.mil/ironbank/jetstack/$1"
      - source: "(.*)"
        replacement: "quay.io/$1"

I’m not sure what the toml side would look like; I’ll have to brush up on whether or not subsections are returned in any sort of order or not, but probably something like:

[plugins.cri.registry.mirrors."quay.io"]
  endpoint = ["https://harbor.internal.com:5000/"]
  [plugins.cri.registry.mirrors."quay.io".rewrites.0]
    source = "strimzi/(.*)"
    replacement = "registry1.dso.mil/ironbank/opensource/strimzi/$1"
  [plugins.cri.registry.mirrors."quay.io".rewrites.1]
    source = "jetstack/(.*)"
    replacement = "registry1.dso.mil/ironbank/jetstack/$1"
  [plugins.cri.registry.mirrors."quay.io".rewrites.2]
    source = "(.*)"
    replacement = "quay.io/$1"

@brandond brandond added this to the Not Scheduled milestone Aug 16, 2022
@brandond
Copy link
Member

Although the ask here isn't exactly the same, the proper way to do this is probably what's discussed at k3s-io/k3s#5568

@tuupola
Copy link

tuupola commented Aug 23, 2023

I also got hit by this with K3s.

$ sudo /usr/local/bin/crictl info | grep docker --after-context 8

"docker.io": {
  "endpoint": [
    "https://harbor.example.com"
  ],
  "rewrite": {
    "^rancher/k3s-upgrade(:v1.(22|23).*)": "k3s-mirror/rancher/k3s-upgrade-backport$1",
    "^rancher/k3s-upgrade(:v1.(24|25|26|27|28).*)": "k3s-mirror/rancher/k3s-upgrade$1"
  }
}

This works fine for the rancher/k3s-upgrade images.

However apparently you need to have a catchall rewrite (.*) to enable installing of other docker.io images from the mirror. Since the order of rewrites is not guaranteed the catchall can become the first rule and the rancher/k3s-upgrade rules will be ignored.

@brandond
Copy link
Member

Yes, you'd need to ensure that the wildcard doesn't match the k3s-upgrade image. It is unfortunate but probably won't be fixed until we switch to the new containerd config directory schema.

@tuupola
Copy link

tuupola commented Aug 24, 2023

I was wrong saying my above example works. The regexp is only for image name and the tag part is not included. So instead of:

"^rancher/k3s-upgrade(.*)": "k3s-mirror/rancher/k3s-upgrade-backport$1"	

I could also use:

"^rancher/k3s-upgrade": "k3s-mirror/rancher/k3s-upgrade-backport"	

Reading the containerd documentation I assume it is not possible to do a rewrite based on image version. It is only possible by image name?

@brandond
Copy link
Member

brandond commented Aug 24, 2023

correct. You're rewriting the repository portion of the image spec. Not the registry, and not the tag.

@xzxiaoshan

This comment was marked as off-topic.

@codering
Copy link

codering commented Nov 1, 2024

How is it now ?

@brandond
Copy link
Member

brandond commented Nov 1, 2024

Same. Not generally a huge issue though, you just have to be strategic with your regexes.

@codering
Copy link

codering commented Nov 1, 2024

I try to use regexes like this

docker.io:
  endpoint:
    - "https://my-registry.me"
  rewrite:
    "repo1/(.*)": "private/$1"
    "repo2/(.*)": "private/$1"
    "^((?!repo1|repo2).+)/(.*)": "public/$1/$2"

then k3s restart failed:

warning msg="Failed to compile rewrite, `^((?!repo1|repo2).+)/(.*)`, for my-registry.me" error="error parsing regexp: invalid or unsupported Perl syntax: `(?!`"

@brandond
Copy link
Member

brandond commented Nov 1, 2024

Check the golang docs on regexes to see what syntax is supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants