Skip to content

Commit 3014118

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2799 from sebsoto/hostTomlChange
OCPBUGS-47696: Fix containerd host.toml generation to work with mirrors using an org_name
2 parents d3f5a46 + fa41815 commit 3014118

File tree

3 files changed

+182
-44
lines changed

3 files changed

+182
-44
lines changed

‎README.md‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,13 +274,25 @@ See install options in the [EC2LaunchV2 documentation](https://docs.aws.amazon.c
274274

275275
In order to run Windows workloads on Nodes, the image `mcr.microsoft.com/oss/kubernetes/pause:3.9` must be mirrored.
276276
See [Image configuration resources](https://docs.openshift.com/container-platform/latest/openshift_images/image-configuration.html) for general information on image mirroring.
277+
Using ImageDigestMirrorSets and ImageTagMirrorSets to mirror container images results in different behavior than Linux Nodes.
278+
Please account for the following differences when reading the above documentation.
277279

278-
Windows images mirrored through ImageDigestMirrorSet and ImageTagMirrorSet have specific naming requirements.
280+
Mirroring on Linux nodes acts on the image level, while on Windows nodes we are only able to mirror on the registry level.
281+
For example, if the cluster has an ImageTagMirrorSet (ITMS) specifying that `quay.io/remote-org/image` should use the mirror `quay.io/my-org/image`
282+
any container that uses `quay.io/remote-org/image:tag` will instead use the image `quay.io/my-org/image:tag`.
283+
284+
This behavior differs for Windows Nodes, which will take the ITMS and use it to configure registry wide mirrors. Following the previous example,
285+
by specifying a mirror of `quay.io/remote-org/image` to `quay.io/my-org/image`, Windows Nodes now use that mirror for all images from `quay.io/remote-org`.
286+
In practice this means that `quay.io/remote-org/image:tag` will use the image `quay.io/my-org/image:tag` as expected, but another container using `quay.io/remote-org/different-image:tag`
287+
will also try to use the mirror `quay.io/remote-org/different-image:tag`. This can cause unintended behavior if it is not accounted for.
288+
For this reason it is recommended to specify container images using a digest, and to use ImageDigestMirrorSets instead of ImageTagMirrorSets.
289+
This can prevent the wrong container image from being used, by ensuring the image the container specifies and the image being pulled have the same digest.
290+
291+
Additionally, Windows images mirrored through ImageDigestMirrorSet and ImageTagMirrorSet have specific naming requirements.
279292
The mirrored image's suffix (final portion of namespaces and the image name) must match that of the source image.
280293
For example, when mirroring the image `mcr.microsoft.com/oss/kubernetes/pause:3.9`, the mirror must have the format
281-
`$mirrorRegistry/[$optionalNamespaces/]oss/kubernetes/pause:3.9` where `$optionalNamespaces` can be any number of
282-
leading namespaces. Some valid values could be: `$mirrorRegistry/oss/kubernetes/pause:3.9`,
283-
`$mirrorRegistry/custom/oss/kubernetes/pause:3.9`, `$mirrorRegistry/x/y/z/oss/kubernetes/pause:3.9`.
294+
`$mirrorRegistry/[$org/]oss/kubernetes/pause:3.9` where `$org` can be any org name, or excluded completely.
295+
Some valid values could be: `$mirrorRegistry/oss/kubernetes/pause:3.9`, `$mirrorRegistry/custom/oss/kubernetes/pause:3.9`, `$mirrorRegistry/x/y/z/oss/kubernetes/pause:3.9`.
284296

285297
### Horizontal Pod Autoscaling
286298
Horizontal Pod autoscaling is available for Windows workloads.

‎pkg/registries/registries.go‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/base64"
66
"encoding/json"
77
"fmt"
8+
"net/url"
89
"sort"
910
"strings"
1011

@@ -41,7 +42,7 @@ func newMirror(sourceImageLocation, mirrorImageLocation string, resolveTags bool
4142
mirrorHost = extractMirrorURL(sourceImageLocation, mirrorImageLocation)
4243
} else {
4344
// special case if source and mirror are the same. Do not drop the host repo name to avoid an empty host entry
44-
mirrorHost = extractHostname(mirrorImageLocation)
45+
mirrorHost = extractRegistryHostname(mirrorImageLocation)
4546
}
4647
return mirror{host: mirrorHost, resolveTags: resolveTags}
4748
}
@@ -82,14 +83,33 @@ func newMirrorSet(srcImage string, mirrorLocations []config.ImageMirror, resolve
8283
for _, m := range mirrorLocations {
8384
truncatedMirrors = append(truncatedMirrors, newMirror(srcImage, string(m), resolveTags))
8485
}
85-
return mirrorSet{source: extractHostname(srcImage), mirrors: truncatedMirrors, mirrorSourcePolicy: mirrorSourcePolicy}
86+
return mirrorSet{source: extractRegistryHostname(srcImage), mirrors: truncatedMirrors, mirrorSourcePolicy: mirrorSourcePolicy}
8687
}
8788

88-
// extractHostname extracts just the initial host repo from a full image location
89-
// e.g. mcr.microsoft.com would be extracted from mcr.microsoft.com/oss/kubernetes/pause:3.9
90-
func extractHostname(fullImage string) string {
91-
parts := strings.Split(fullImage, imagePathSeparator)
92-
return parts[0]
89+
// extractRegistryHostname extracts just the initial host repo from a full image location, as containerd does not allow
90+
// registries to exist on a subpath, given an input image `mcr.microsoft.com/oss/kubernetes/pause:3.9`,
91+
// mcr.microsoft.com would be the determined registry hostname.
92+
func extractRegistryHostname(fullImage string) string {
93+
// url.Parse will only work if URL has a scheme (https://)
94+
if parsedURL, err := url.Parse(fullImage); err == nil && parsedURL.Hostname() != "" {
95+
if parsedURL.Port() != "" {
96+
return parsedURL.Hostname() + ":" + parsedURL.Port()
97+
}
98+
return parsedURL.Hostname()
99+
}
100+
// For URLs without a scheme, just return everything before the first `/`
101+
return strings.Split(fullImage, imagePathSeparator)[0]
102+
}
103+
104+
// extractRegistryOrgPath returns only the org path when given a reference to a registry.
105+
// The input for this function should not include an image name.
106+
func extractRegistryOrgPath(registry string) string {
107+
hostname := extractRegistryHostname(registry)
108+
hostnameSplit := strings.SplitN(registry, hostname, 2)
109+
if len(hostnameSplit) != 2 {
110+
return ""
111+
}
112+
return strings.TrimPrefix(hostnameSplit[1], "/")
93113
}
94114

95115
// getMergedMirrorSets extracts and merges the contents of the given mirror sets.
@@ -209,14 +229,24 @@ func (ms *mirrorSet) generateConfig(secretsConfig credentialprovider.DockerConfi
209229
// set the fallback server to the first mirror to ensure the source is never contacted, even if all mirrors fail
210230
fallbackServer = ms.mirrors[0].host
211231
}
212-
result += fmt.Sprintf("server = \"https://%s\"", fallbackServer)
213-
result += "\r\n"
232+
fallbackRegistry := extractRegistryHostname(fallbackServer)
233+
result += fmt.Sprintf("server = \"https://%s/v2", fallbackRegistry)
234+
if orgPath := extractRegistryOrgPath(fallbackServer); orgPath != "" {
235+
result += "/" + orgPath
236+
}
237+
result += "\"\n"
238+
result += "\noverride_path = true\n"
214239

215240
// Each mirror should result in an entry followed by a set of settings for interacting with the mirror host
216241
for _, m := range ms.mirrors {
217-
result += "\r\n"
218-
result += fmt.Sprintf("[host.\"https://%s\"]", m.host)
219-
result += "\r\n"
242+
hostRegistry := extractRegistryHostname(m.host)
243+
hostOrgPath := extractRegistryOrgPath(m.host)
244+
result += "\n"
245+
result += fmt.Sprintf("[host.\"https://%s/v2", hostRegistry)
246+
if hostOrgPath != "" {
247+
result += "/" + hostOrgPath
248+
}
249+
result += "\"]\n"
220250

221251
// Specify the operations the registry host may perform. IDMS mirrors can only be pulled by directly by digest,
222252
// whereas ITMS mirrors have the additional resolve capability, which allows converting a tag name into a digest
@@ -227,18 +257,22 @@ func (ms *mirrorSet) generateConfig(secretsConfig credentialprovider.DockerConfi
227257
hostCapabilities = " capabilities = [\"pull\"]"
228258
}
229259
result += hostCapabilities
230-
result += "\r\n"
260+
result += "\n"
261+
result += " override_path = true\n"
231262

232263
// Extract the mirror repo's authorization credentials, if one exists
233-
if entry, ok := secretsConfig.Auths[extractHostname(m.host)]; ok {
264+
if entry, ok := secretsConfig.Auths[extractRegistryHostname(m.host)]; ok {
234265
credentials := entry.Username + ":" + entry.Password
235266
token := base64.StdEncoding.EncodeToString([]byte(credentials))
236267

237268
// Add the access token as a request header
238-
result += fmt.Sprintf(" [host.\"https://%s\".header]", m.host)
239-
result += "\r\n"
269+
result += fmt.Sprintf(" [host.\"https://%s/v2", hostRegistry)
270+
if hostOrgPath != "" {
271+
result += "/" + hostOrgPath
272+
}
273+
result += "\".header]\n"
240274
result += fmt.Sprintf(" authorization = \"Basic %s\"", token)
241-
result += "\r\n"
275+
result += "\n"
242276
}
243277
}
244278

‎pkg/registries/registries_test.go‎

Lines changed: 115 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
7979
Spec: config.ImageDigestMirrorSetSpec{
8080
ImageDigestMirrors: []config.ImageDigestMirrors{
8181
{
82-
Source: "source1",
82+
Source: "https://source1.local:5000",
8383
Mirrors: []config.ImageMirror{"mirror1"},
8484
MirrorSourcePolicy: config.AllowContactingSource,
8585
},
@@ -102,7 +102,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
102102
},
103103
expectedOutput: []mirrorSet{
104104
{
105-
source: "source1",
105+
source: "source1.local:5000",
106106
mirrors: []mirror{{host: "mirror1", resolveTags: false}},
107107
mirrorSourcePolicy: config.AllowContactingSource,
108108
},
@@ -133,7 +133,7 @@ func TestGetMergedMirrorSets(t *testing.T) {
133133
Spec: config.ImageTagMirrorSetSpec{
134134
ImageTagMirrors: []config.ImageTagMirrors{
135135
{
136-
Source: "mcr.microsoft.com/oss/kubernetes/pause",
136+
Source: "docker://mcr.microsoft.com/oss/kubernetes/pause",
137137
Mirrors: []config.ImageMirror{"quay.io/testuser/oss/kubernetes/pause"},
138138
MirrorSourcePolicy: config.AllowContactingSource,
139139
},
@@ -237,7 +237,7 @@ func TestGenerateConfig(t *testing.T) {
237237
{
238238
name: "empty mirrors",
239239
input: mirrorSet{
240-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
240+
source: "registry.access.redhat.com/ubi9",
241241
mirrors: []mirror{},
242242
mirrorSourcePolicy: config.AllowContactingSource,
243243
},
@@ -246,72 +246,138 @@ func TestGenerateConfig(t *testing.T) {
246246
{
247247
name: "basic one digest mirror",
248248
input: mirrorSet{
249-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
249+
source: "registry.access.redhat.com",
250250
mirrors: []mirror{
251-
{host: "example.io/example/ubi-minimal", resolveTags: false},
251+
{host: "example.io/example", resolveTags: false},
252252
},
253253
mirrorSourcePolicy: config.AllowContactingSource,
254254
},
255-
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n",
255+
expectedOutput: `server = "https://registry.access.redhat.com/v2"
256+
257+
override_path = true
258+
259+
[host."https://example.io/v2/example"]
260+
capabilities = ["pull"]
261+
override_path = true
262+
`,
256263
},
257264
{
258265
name: "basic one tag mirror",
259266
input: mirrorSet{
260-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
267+
source: "registry.access.redhat.com",
261268
mirrors: []mirror{
262-
{host: "example.io/example/ubi-minimal", resolveTags: true},
269+
{host: "example.io/example", resolveTags: true},
263270
},
264271
mirrorSourcePolicy: config.AllowContactingSource,
265272
},
266-
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n",
273+
expectedOutput: `server = "https://registry.access.redhat.com/v2"
274+
275+
override_path = true
276+
277+
[host."https://example.io/v2/example"]
278+
capabilities = ["pull", "resolve"]
279+
override_path = true
280+
`,
267281
},
268282
{
269283
name: "one digest mirror never contact source",
270284
input: mirrorSet{
271-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
285+
source: "registry.access.redhat.com",
272286
mirrors: []mirror{
273-
{host: "example.io/example/ubi-minimal", resolveTags: false},
287+
{host: "example.io/example", resolveTags: false},
274288
},
275289
mirrorSourcePolicy: config.NeverContactSource,
276290
},
277-
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n",
291+
expectedOutput: `server = "https://example.io/v2/example"
292+
293+
override_path = true
294+
295+
[host."https://example.io/v2/example"]
296+
capabilities = ["pull"]
297+
override_path = true
298+
`,
278299
},
279300
{
280301
name: "tags mirror never contact source",
281302
input: mirrorSet{
282-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
303+
source: "registry.access.redhat.com",
283304
mirrors: []mirror{
284-
{host: "example.io/example/ubi-minimal", resolveTags: true},
305+
{host: "example.io/example", resolveTags: true},
285306
},
286307
mirrorSourcePolicy: config.NeverContactSource,
287308
},
288-
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n",
309+
expectedOutput: `server = "https://example.io/v2/example"
310+
311+
override_path = true
312+
313+
[host."https://example.io/v2/example"]
314+
capabilities = ["pull", "resolve"]
315+
override_path = true
316+
`,
289317
},
290318
{
291319
name: "multiple mirrors",
292320
input: mirrorSet{
293-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
321+
source: "registry.access.redhat.com",
294322
mirrors: []mirror{
295-
{host: "example.io/example/ubi-minimal", resolveTags: false},
323+
{host: "example.io/example", resolveTags: false},
296324
{host: "mirror.example.com/redhat", resolveTags: false},
297-
{host: "mirror.example.net/image", resolveTags: true},
325+
{host: "mirror.example.net", resolveTags: true},
298326
},
299327
mirrorSourcePolicy: config.AllowContactingSource,
300328
},
301-
expectedOutput: "server = \"https://registry.access.redhat.com/ubi9/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n\r\n[host.\"https://mirror.example.com/redhat\"]\r\n capabilities = [\"pull\"]\r\n [host.\"https://mirror.example.com/redhat\".header]\r\n authorization = \"Basic dXNlcjpwYXNz\"\r\n\r\n[host.\"https://mirror.example.net/image\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n [host.\"https://mirror.example.net/image\".header]\r\n authorization = \"Basic dXNlcm5hbWU6cGFzc3dvcmQ=\"\r\n",
329+
expectedOutput: `server = "https://registry.access.redhat.com/v2"
330+
331+
override_path = true
332+
333+
[host."https://example.io/v2/example"]
334+
capabilities = ["pull"]
335+
override_path = true
336+
337+
[host."https://mirror.example.com/v2/redhat"]
338+
capabilities = ["pull"]
339+
override_path = true
340+
[host."https://mirror.example.com/v2/redhat".header]
341+
authorization = "Basic dXNlcjpwYXNz"
342+
343+
[host."https://mirror.example.net/v2"]
344+
capabilities = ["pull", "resolve"]
345+
override_path = true
346+
[host."https://mirror.example.net/v2".header]
347+
authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="
348+
`,
302349
},
303350
{
304351
name: "multiple mirrors never contact source",
305352
input: mirrorSet{
306-
source: "registry.access.redhat.com/ubi9/ubi-minimal",
353+
source: "registry.access.redhat.com",
307354
mirrors: []mirror{
308-
{host: "example.io/example/ubi-minimal", resolveTags: false},
355+
{host: "example.io/example", resolveTags: false},
309356
{host: "mirror.example.com/redhat", resolveTags: false},
310357
{host: "mirror.example.net", resolveTags: true},
311358
},
312359
mirrorSourcePolicy: config.NeverContactSource,
313360
},
314-
expectedOutput: "server = \"https://example.io/example/ubi-minimal\"\r\n\r\n[host.\"https://example.io/example/ubi-minimal\"]\r\n capabilities = [\"pull\"]\r\n\r\n[host.\"https://mirror.example.com/redhat\"]\r\n capabilities = [\"pull\"]\r\n [host.\"https://mirror.example.com/redhat\".header]\r\n authorization = \"Basic dXNlcjpwYXNz\"\r\n\r\n[host.\"https://mirror.example.net\"]\r\n capabilities = [\"pull\", \"resolve\"]\r\n [host.\"https://mirror.example.net\".header]\r\n authorization = \"Basic dXNlcm5hbWU6cGFzc3dvcmQ=\"\r\n",
361+
expectedOutput: `server = "https://example.io/v2/example"
362+
363+
override_path = true
364+
365+
[host."https://example.io/v2/example"]
366+
capabilities = ["pull"]
367+
override_path = true
368+
369+
[host."https://mirror.example.com/v2/redhat"]
370+
capabilities = ["pull"]
371+
override_path = true
372+
[host."https://mirror.example.com/v2/redhat".header]
373+
authorization = "Basic dXNlcjpwYXNz"
374+
375+
[host."https://mirror.example.net/v2"]
376+
capabilities = ["pull", "resolve"]
377+
override_path = true
378+
[host."https://mirror.example.net/v2".header]
379+
authorization = "Basic dXNlcm5hbWU6cGFzc3dvcmQ="
380+
`,
315381
},
316382
}
317383

@@ -637,3 +703,29 @@ func TestExtractMirrorURL(t *testing.T) {
637703
})
638704
}
639705
}
706+
707+
func TestExtractOrgPath(t *testing.T) {
708+
tests := []struct {
709+
input string
710+
expected string
711+
}{
712+
{
713+
input: "registry.local/org",
714+
expected: "org",
715+
},
716+
{
717+
input: "registry.local/org/sub_org",
718+
expected: "org/sub_org",
719+
},
720+
{
721+
input: "registry.local",
722+
expected: "",
723+
},
724+
}
725+
for _, tt := range tests {
726+
t.Run(tt.input, func(t *testing.T) {
727+
result := extractRegistryOrgPath(tt.input)
728+
assert.Equal(t, tt.expected, result)
729+
})
730+
}
731+
}

0 commit comments

Comments
 (0)