Repository: kubermatic/machine-controller
Pull request #1467 (2022-10-24 11:39:42)

Extend containerd registry mirror config
**What this PR does / why we need it**:

containerd can be configured for multiple registry mirrors per registry, this extends machine-controller to configure mirrors for registries other than docker.io, reusing the existing command line flag.

**What type of PR is this?**
/kind feature

**Special notes for your reviewer**:

This should probably be documented, but didn't find where..

**Does this PR introduce a user-facing change? Then add your Release Note here**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Allow configuring containerd registry mirrors not only for docker.io
```

**Documentation**:
<!--
Please do one of the following options:
- Add a link to the existing documentation
- Add a link to the kubermatic/docs pull request
- If no documentation change is applicable then add:
  - TBD (documentation will be added later)
  - NONE (no documentation needed for this PR)
-->
```documentation
NONE
```


kubermatic-bot: @LittleFox94: Adding the "do-not-merge/docs-needed" label because no documentation block was detected, please follow our [documentation process](https://github.com/kubermatic/docs/blob/master/PROCESS.md) to remove it.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1467#" title="Author self-approved">LittleFox94</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign embik for approval by writing `/assign @embik` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["embik"]} -->
kubermatic-bot: Hi @LittleFox94. Thanks for your PR.

I'm waiting for a [kubermatic](https://github.com/orgs/kubermatic/people) 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](https://github.com/orgs/kubermatic/people) 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](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>


Patch:
diff --git a/pkg/cloudprovider/provider/aws/types/cloudconfig.go b/pkg/cloudprovider/provider/aws/types/cloudconfig.go
index f7076153..2fca4788 100644
--- a/pkg/cloudprovider/provider/aws/types/cloudconfig.go
+++ b/pkg/cloudprovider/provider/aws/types/cloudconfig.go
@@ -37,6 +37,9 @@ KubernetesClusterID={{ .Global.KubernetesClusterID | iniEscape }}
 DisableSecurityGroupIngress={{ .Global.DisableSecurityGroupIngress }}
 ElbSecurityGroup={{ .Global.ElbSecurityGroup | iniEscape }}
 DisableStrictZoneCheck={{ .Global.DisableStrictZoneCheck }}
+{{- range .Global.NodeIPFamilies }}
+NodeIPFamilies={{ . | iniEscape}}
+{{- end }}
 `
 )
 
@@ -55,6 +58,7 @@ type GlobalOpts struct {
 	ElbSecurityGroup            string
 	DisableSecurityGroupIngress bool
 	DisableStrictZoneCheck      bool
+	NodeIPFamilies              []string
 }
 
 func CloudConfigToString(c *CloudConfig) (string, error) {
diff --git a/pkg/containerruntime/config.go b/pkg/containerruntime/config.go
index 551befbb..90bce030 100644
--- a/pkg/containerruntime/config.go
+++ b/pkg/containerruntime/config.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/url"
+	"regexp"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
@@ -45,11 +46,25 @@ func BuildConfig(opts Opts) (Config, error) {
 		}
 	}
 
-	var registryMirrors []string
+	// we want to match e.g. docker.io=registry.docker-cn.com, having docker.io as the first
+	// match group and registry.docker-cn.com as the second one.
+	registryMirrorRegexp := regexp.MustCompile(`^([a-zA-Z0-9\.-]+)=(.*)`)
+
+	if opts.ContainerdRegistryMirrors == nil {
+		opts.ContainerdRegistryMirrors = make(RegistryMirrorsFlags)
+	}
+
 	for _, mirror := range strings.Split(opts.RegistryMirrors, ",") {
 		if trimmedMirror := strings.TrimSpace(mirror); trimmedMirror != "" {
-			if !strings.HasPrefix(mirror, "http") {
-				trimmedMirror = "https://" + mirror
+			registry := "docker.io"
+
+			if matches := registryMirrorRegexp.FindStringSubmatch(trimmedMirror); matches != nil {
+				registry = matches[1]
+				trimmedMirror = matches[2]
+			}
+
+			if !strings.HasPrefix(trimmedMirror, "http") {
+				trimmedMirror = "https://" + trimmedMirror
 			}
 
 			_, err := url.Parse(trimmedMirror)
@@ -57,15 +72,12 @@ func BuildConfig(opts Opts) (Config, error) {
 				return Config{}, fmt.Errorf("incorrect mirror provided: %w", err)
 			}
 
-			registryMirrors = append(registryMirrors, trimmedMirror)
-		}
-	}
+			if opts.ContainerdRegistryMirrors[registry] == nil {
+				opts.ContainerdRegistryMirrors[registry] = make([]string, 0, 1)
+			}
 
-	if len(registryMirrors) > 0 {
-		if opts.ContainerdRegistryMirrors == nil {
-			opts.ContainerdRegistryMirrors = make(RegistryMirrorsFlags)
+			opts.ContainerdRegistryMirrors[registry] = append(opts.ContainerdRegistryMirrors[registry], trimmedMirror)
 		}
-		opts.ContainerdRegistryMirrors["docker.io"] = registryMirrors
 	}
 
 	// Only validate registry credential here


Test patch:
diff --git a/pkg/cloudprovider/provider/aws/types/cloudconfig_test.go b/pkg/cloudprovider/provider/aws/types/cloudconfig_test.go
index ed6423d2..f9eaa1cf 100644
--- a/pkg/cloudprovider/provider/aws/types/cloudconfig_test.go
+++ b/pkg/cloudprovider/provider/aws/types/cloudconfig_test.go
@@ -46,6 +46,7 @@ func TestCloudConfigToString(t *testing.T) {
 					KubernetesClusterTag:        "some-tag",
 					RoleARN:                     "some-arn",
 					RouteTableID:                "some-rt",
+					NodeIPFamilies:              []string{"ipv4", "ipv6"},
 				},
 			},
 		},
diff --git a/pkg/cloudprovider/provider/aws/types/testdata/simple-config.golden b/pkg/cloudprovider/provider/aws/types/testdata/simple-config.golden
index 4642120f..57bffe19 100644
--- a/pkg/cloudprovider/provider/aws/types/testdata/simple-config.golden
+++ b/pkg/cloudprovider/provider/aws/types/testdata/simple-config.golden
@@ -8,3 +8,5 @@ KubernetesClusterID="some-tag"
 DisableSecurityGroupIngress=true
 ElbSecurityGroup="some-sg"
 DisableStrictZoneCheck=true
+NodeIPFamilies="ipv4"
+NodeIPFamilies="ipv6"
diff --git a/pkg/containerruntime/config_test.go b/pkg/containerruntime/config_test.go
new file mode 100644
index 00000000..4ee6ecd7
--- /dev/null
+++ b/pkg/containerruntime/config_test.go
@@ -0,0 +1,134 @@
+/*
+Copyright 2022 The Machine Controller Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package containerruntime
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+)
+
+func TestContainerdRegistryMirror(t *testing.T) {
+	type testCase struct {
+		desc            string
+		flag            string
+		expectedMirrors map[string][]string
+		expectedError   error
+	}
+
+	testCases := []testCase{
+		{
+			desc:            "no registry mirrors set",
+			flag:            "",
+			expectedMirrors: map[string][]string{},
+			expectedError:   nil,
+		},
+
+		{
+			desc: "registry mirror without name and protocol",
+			flag: "registry-v1.docker.io",
+			expectedMirrors: map[string][]string{
+				"docker.io": {"https://registry-v1.docker.io"},
+			},
+			expectedError: nil,
+		},
+		{
+			desc: "multiple registry mirrors without name, with and without protocol",
+			flag: "registry-v1.docker.io,http://registry.docker-cn.com",
+			expectedMirrors: map[string][]string{
+				"docker.io": {
+					"https://registry-v1.docker.io",
+					"http://registry.docker-cn.com",
+				},
+			},
+			expectedError: nil,
+		},
+
+		{
+			desc: "registry mirror with name and without protocol",
+			flag: "quay.io=my-quay-io-mirror.example.com",
+			expectedMirrors: map[string][]string{
+				"quay.io": {"https://my-quay-io-mirror.example.com"},
+			},
+			expectedError: nil,
+		},
+		{
+			desc: "registry mirror with name and protocol",
+			flag: "quay.io=http://my-quay-io-mirror.example.com",
+			expectedMirrors: map[string][]string{
+				"quay.io": {"http://my-quay-io-mirror.example.com"},
+			},
+			expectedError: nil,
+		},
+		{
+			desc: "multiple registry mirrors with same name",
+			flag: "quay.io=http://my-quay-io-mirror.example.com,quay.io=example.net",
+			expectedMirrors: map[string][]string{
+				"quay.io": {
+					"http://my-quay-io-mirror.example.com",
+					"https://example.net",
+				},
+			},
+			expectedError: nil,
+		},
+
+		{
+			desc: "complex example",
+			flag: "quay.io=http://my-quay-io-mirror.example.com,quay.io=example.net," +
+				"registry-v1.docker.io,http://registry.docker-cn.com," +
+				"ghcr.io=http://foo/bar",
+			expectedMirrors: map[string][]string{
+				"quay.io": {
+					"http://my-quay-io-mirror.example.com",
+					"https://example.net",
+				},
+				"docker.io": {
+					"https://registry-v1.docker.io",
+					"http://registry.docker-cn.com",
+				},
+				"ghcr.io": {
+					"http://foo/bar",
+				},
+			},
+			expectedError: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			opts := Opts{
+				ContainerRuntime: containerdName,
+				RegistryMirrors:  tc.flag,
+			}
+
+			config, err := BuildConfig(opts)
+			if tc.expectedError != nil {
+				if !errors.Is(err, tc.expectedError) {
+					t.Errorf("expected error %q but got %q", tc.expectedError, err)
+				}
+			}
+
+			if err != nil {
+				t.Errorf("expected success but got error: %q", err)
+			}
+
+			if fmt.Sprint(config.RegistryMirrors) != fmt.Sprint(tc.expectedMirrors) {
+				t.Errorf("expected to get %v instead got: %v", tc.expectedMirrors, config.RegistryMirrors)
+			}
+		})
+	}
+}