Skip to content

Commit

Permalink
Use MCO and CCO image references when looking up mappings
Browse files Browse the repository at this point in the history 
Currently, we unconditionally use an image mapping from the management
cluster if a mapping exists for ocp-release-dev or ocp/release.
When the individual images do not use those registries, the wrong
mapping is used.
This commit uses the individual images from the payload to look for any
image mappings that may exist on the management cluster.
  • Loading branch information
@csrwng
csrwng committed Sep 6, 2023
1 parent 068f642 commit 18ff7de
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 74 deletions.
86 changes: 35 additions & 51 deletions control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver.go
View file
Expand Up @@ -5,7 +5,6 @@ import (
"context"
"fmt"
"net"
"strings"

configv1 "github.com/openshift/api/config/v1"
routev1 "github.com/openshift/api/route/v1"
Expand Down Expand Up @@ -186,53 +185,40 @@ func ReconcileIgnitionServer(ctx context.Context,
if configOperatorImage == "" {
return fmt.Errorf("cluster-config-operator image not found in payload images")
}
machineConfigOperatorImage := componentImages["machine-config-operator"]
if machineConfigOperatorImage == "" {
return fmt.Errorf("machine-config-operator image not found in payload images")
}
servingCertSecretName := ignitionserver.IgnitionServingCertSecret("").Name
if hcp.Spec.Platform.Type != hyperv1.IBMCloudPlatform {
servingCertSecretName = manifests.IgnitionServerCertSecret("").Name
}

var (
icsFound bool
sourceRef reference.DockerImageReference
mirrorImage string
)

if openShiftRegistryOverrides != "=" && len(openShiftRegistryOverrides) > 0 {
var err error
// Ignition server cannot handle ImageContentSourcePolicies or ImageDigestMachineSet, so we need to
// fill the registryOverrides in the case of disconnected environments
mirrorImage = lookupDisconnectedRegistry(ctx, openShiftRegistryOverrides)
if len(mirrorImage) > 0 {
sourceRef, err = reference.Parse(mirrorImage)
if err != nil {
return fmt.Errorf("failed to parse private registry hosted control plane image reference %q: %w", mirrorImage, err)
}
icsFound = true
}
// Determine if we need to override the machine config operator and cluster config operator
// images based on image mappings present in management cluster.
ocpRegistryMapping := util.ConvertImageRegistryOverrideStringToMap(openShiftRegistryOverrides)
overrideConfigOperatorImage, err := lookupMappedImage(ocpRegistryMapping, configOperatorImage)
if err != nil {
return err
}
overrideMachineConfigOperatorImage, err := lookupMappedImage(ocpRegistryMapping, machineConfigOperatorImage)
if err != nil {
return err
}

if icsFound {
privateRegistry := sourceRef.Registry

if !strings.HasPrefix(componentImages["machine-config-operator"], privateRegistry) {
ocpReleaseMcoImage := componentImages["machine-config-operator"]
mcoSha, err := reference.Parse(ocpReleaseMcoImage)
if err != nil {
return fmt.Errorf("failed to parse machine-config-operator image reference %q: %w", ocpReleaseMcoImage, err)
}
registryOverrides[ocpReleaseMcoImage] = fmt.Sprintf("%s@%s", mirrorImage, mcoSha.ID)
imageOverrides := map[string]string{}
for k, v := range registryOverrides {
if k != "" {
imageOverrides[k] = v
}
}

if !strings.HasPrefix(componentImages["cluster-config-operator"], privateRegistry) {
ocpReleaseCCOImage := componentImages["cluster-config-operator"]
ccoSha, err := reference.Parse(ocpReleaseCCOImage)
if err != nil {
return fmt.Errorf("failed to parse cluster-config-operator image reference %s: %w", ocpReleaseCCOImage, err)
}
registryOverrides[ocpReleaseCCOImage] = fmt.Sprintf("%s@%s", mirrorImage, ccoSha.ID)
}
if overrideConfigOperatorImage != configOperatorImage {
imageOverrides[configOperatorImage] = overrideConfigOperatorImage
}

delete(registryOverrides, "")
if overrideMachineConfigOperatorImage != machineConfigOperatorImage {
imageOverrides[machineConfigOperatorImage] = overrideMachineConfigOperatorImage
}

ignitionServerDeployment := ignitionserver.Deployment(controlPlaneNamespace)
Expand All @@ -244,7 +230,7 @@ func ReconcileIgnitionServer(ctx context.Context,
hcp,
defaultIngressDomain,
hasHealthzHandler,
registryOverrides,
imageOverrides,
openShiftRegistryOverrides,
managementClusterHasCapabilitySecurityContextConstraint,
ignitionServerLabels,
Expand Down Expand Up @@ -896,18 +882,16 @@ cp /tmp/manifests/99_feature-gate.yaml %[1]s/99_feature-gate.yaml
return fmt.Sprintf(script, workDir, featureGateYAML)
}

func lookupDisconnectedRegistry(ctx context.Context, strOcpOverrides string) string {
ocpOverrides := strings.Split(strOcpOverrides, ",")
for _, entry := range ocpOverrides {
sourceImage := strings.Split(entry, "=")[0]
mirrorImage := strings.Split(entry, "=")[1]

if strings.Contains(sourceImage, "openshift-release-dev") || strings.Contains(sourceImage, "ocp/release") {
// Production releases: 'openshift-release-dev'
// Nightly releases: 'ocp/release'
return mirrorImage
func lookupMappedImage(ocpOverrides map[string][]string, image string) (string, error) {
ref, err := reference.Parse(image)
if err != nil {
return "", fmt.Errorf("failed to parse image (%s): %w", image, err)
}
for source, replacements := range ocpOverrides {
if ref.AsRepository().String() == source {
newRef := fmt.Sprintf("%s@%s", replacements[0], ref.ID)
return newRef, nil
}
}

return ""
return image, nil
}
53 changes: 30 additions & 23 deletions control-plane-operator/controllers/hostedcontrolplane/ignitionserver/ignitionserver_test.go
View file
@@ -1,8 +1,6 @@
package ignitionserver

import (
"context"
"reflect"
"testing"

. "github.com/onsi/gomega"
Expand Down Expand Up @@ -143,37 +141,46 @@ func TestReconcileIgnitionServerServiceRoute(t *testing.T) {
}
}

func TestLookupDisconnectedRegistry(t *testing.T) {
type args struct {
ctx context.Context
ocpOverrides string
func TestLookupMappedImage(t *testing.T) {

ocpOverrides := map[string][]string{
"quay.io/jparrill": {"registry.hypershiftbm.lab:5000/jparrill"},
"quay.io/karmab": {"registry.hypershiftbm.lab:5000/karmab"},
"quay.io/mavazque": {"registry.hypershiftbm.lab:5000/mavazque"},
"quay.io/ocp-release": {"registry.hypershiftbm.lab:5000/openshift/release-images"},
"quay.io/openshift-release-dev/ocp-v4.0-art-dev": {"registry.hypershiftbm.lab:5000/openshift/release"},
"registry.access.redhat.com/openshift4/ose-oauth-proxy": {"registry.hypershiftbm.lab:5000/openshift4/ose-oauth-proxy"},
"registry.redhat.io/lvms4": {"registry.hypershiftbm.lab:5000/lvms4"},
"registry.redhat.io/multicluster-engine": {"registry.hypershiftbm.lab:5000/acm-d"},
"registry.redhat.io/openshift4": {"registry.hypershiftbm.lab:5000/openshift4"},
"registry.redhat.io/rhacm2": {"registry.hypershiftbm.lab:5000/acm-d"},
"registry.redhat.io/rhel8": {"registry.hypershiftbm.lab:5000/rhel8"},
}

tests := []struct {
name string
args args
want string
name string
in string
expect string
}{
{
name: "given an wellformed openshiftRegistryOverrides string, it should return the mirrorImage pointing to the private registry",
args: args{
ctx: context.TODO(),
ocpOverrides: "quay.io/jparrill=registry.hypershiftbm.lab:5000/jparrill,quay.io/karmab=registry.hypershiftbm.lab:5000/karmab,quay.io/mavazque=registry.hypershiftbm.lab:5000/mavazque,quay.io/ocp-release=registry.hypershiftbm.lab:5000/openshift/release-images,quay.io/openshift-release-dev/ocp-v4.0-art-dev=registry.hypershiftbm.lab:5000/openshift/release,registry.access.redhat.com/openshift4/ose-oauth-proxy=registry.hypershiftbm.lab:5000/openshift4/ose-oauth-proxy,registry.redhat.io/lvms4=registry.hypershiftbm.lab:5000/lvms4,registry.redhat.io/multicluster-engine=registry.hypershiftbm.lab:5000/acm-d,registry.redhat.io/openshift4=registry.hypershiftbm.lab:5000/openshift4,registry.redhat.io/openshift4=registry.hypershiftbm.lab:5000/openshift4,registry.redhat.io/rhacm2=registry.hypershiftbm.lab:5000/acm-d,registry.redhat.io/rhel8=registry.hypershiftbm.lab:5000/rhel8",
},
want: "registry.hypershiftbm.lab:5000/openshift/release",
name: "return the mirrorImage if it exists in the ocp overrides",
in: "quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:34acb17e45b80267919ea6a2f0e2b21e3f87db3c1e2443d2b3ac9afc0455ae07",
expect: "registry.hypershiftbm.lab:5000/openshift/release@sha256:34acb17e45b80267919ea6a2f0e2b21e3f87db3c1e2443d2b3ac9afc0455ae07",
},
{
name: "given an wellformed openshiftRegistryOverrides string without ocpRelease entry, it should return an empty slice",
args: args{
ctx: context.TODO(),
ocpOverrides: "quay.io/jparrill=registry.hypershiftbm.lab:5000/jparrill,quay.io/karmab=registry.hypershiftbm.lab:5000/karmab,quay.io/mavazque=registry.hypershiftbm.lab:5000/mavazque,registry.access.redhat.com/openshift4/ose-oauth-proxy=registry.hypershiftbm.lab:5000/openshift4/ose-oauth-proxy,registry.redhat.io/lvms4=registry.hypershiftbm.lab:5000/lvms4,registry.redhat.io/multicluster-engine=registry.hypershiftbm.lab:5000/acm-d,registry.redhat.io/openshift4=registry.hypershiftbm.lab:5000/openshift4,registry.redhat.io/openshift4=registry.hypershiftbm.lab:5000/openshift4,registry.redhat.io/rhacm2=registry.hypershiftbm.lab:5000/acm-d,registry.redhat.io/rhel8=registry.hypershiftbm.lab:5000/rhel8",
},
want: "",
name: "return the same image if not in the overrides",
in: "registry.ci.openshift.org/ocp/4.14-2023-09-05-120503@sha256:34acb17e45b80267919ea6a2f0e2b21e3f87db3c1e2443d2b3ac9afc0455ae07",
expect: "registry.ci.openshift.org/ocp/4.14-2023-09-05-120503@sha256:34acb17e45b80267919ea6a2f0e2b21e3f87db3c1e2443d2b3ac9afc0455ae07",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := lookupDisconnectedRegistry(tt.args.ctx, tt.args.ocpOverrides); !reflect.DeepEqual(got, tt.want) {
t.Errorf("lookupDisconnectedRegistry() = %v, want %v", got, tt.want)
got, err := lookupMappedImage(ocpOverrides, tt.in)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if got != tt.expect {
t.Errorf("Wanted: %s, Got: %s", tt.expect, got)
}
})
}
Expand Down

0 comments on commit 18ff7de

Please sign in to comment.