Skip to content

Commit

Permalink
MGMT-17194: Ensure releaseImage is not tag based when mirror registry…
Browse files Browse the repository at this point in the history
… enabled. (#6260)

When mirror registries are enabled, it is not possible to use a tag based image (for example quay.io/openshift-release-dev/ocp-release:4.15.0-multi) for the release image of a cluster being deployed by assisted.
The user should use digest a based image (such as quay.io/openshift-release-dev/ocp-release@sha256:b86422e972b9c838dfdb8b481a67ae08308437d6489ea6aaf150242b1d30fa1c)

This PR ensures that we check for this scenario during the validation of the ClusterDeployment.

This will be propagated to the ClusterDeployments_Controller as a validation failure if the image is tag based.
A spec Sync failure will be indicated in the conditions of the controller.
  • Loading branch information
paul-maidment committed May 5, 2024
1 parent 75eedfc commit bc33455
Show file tree
Hide file tree
Showing 12 changed files with 1,256 additions and 59 deletions.
29 changes: 15 additions & 14 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -579,20 +579,21 @@ func main() {
cluster_client := ctrlMgr.GetClient()
cluster_reader := ctrlMgr.GetAPIReader()
failOnError((&controllers.ClusterDeploymentsReconciler{
Client: cluster_client,
APIReader: cluster_reader,
Log: log,
Scheme: ctrlMgr.GetScheme(),
Installer: bm,
ClusterApi: clusterApi,
HostApi: hostApi,
CRDEventsHandler: crdEventsHandler,
Manifests: manifestsApi,
ServiceBaseURL: Options.BMConfig.ServiceBaseURL,
PullSecretHandler: controllers.NewPullSecretHandler(cluster_client, cluster_reader, bm),
AuthType: Options.Auth.AuthType,
VersionsHandler: versionHandler,
SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log),
Client: cluster_client,
APIReader: cluster_reader,
Log: log,
Scheme: ctrlMgr.GetScheme(),
Installer: bm,
ClusterApi: clusterApi,
HostApi: hostApi,
CRDEventsHandler: crdEventsHandler,
Manifests: manifestsApi,
ServiceBaseURL: Options.BMConfig.ServiceBaseURL,
PullSecretHandler: controllers.NewPullSecretHandler(cluster_client, cluster_reader, bm),
AuthType: Options.Auth.AuthType,
VersionsHandler: versionHandler,
SpokeK8sClientFactory: spoke_k8s_client.NewSpokeK8sClientFactory(log),
MirrorRegistriesConfigBuilder: mirrorregistries.New(),
}).SetupWithManager(ctrlMgr), "unable to create controller ClusterDeployment")

failOnError((&controllers.AgentReconciler{
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ require (
github.com/openshift/generic-admission-server v1.14.1-0.20231020105858-8dcc3c9b298f
github.com/openshift/hive/apis v0.0.0-20220222213051-def9088fdb5a
github.com/openshift/image-customization-controller v0.0.0-20240129110832-60a3867d7f9e
github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/pelletier/go-toml v1.9.5
github.com/pkg/errors v0.9.1
Expand Down Expand Up @@ -126,7 +127,6 @@ require (
github.com/moby/sys/user v0.1.0 // indirect
github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect
github.com/morikuni/aec v1.0.0 // indirect
github.com/openshift/library-go v0.0.0-20231110170715-08d73a9c798b // indirect
github.com/peterbourgon/diskv v2.0.1+incompatible // indirect
github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
Expand Down
53 changes: 46 additions & 7 deletions internal/controller/controllers/clusterdeployments_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,11 @@ import (
"github.com/openshift/assisted-service/models"
"github.com/openshift/assisted-service/pkg/auth"
logutil "github.com/openshift/assisted-service/pkg/log"
"github.com/openshift/assisted-service/pkg/mirrorregistries"
"github.com/openshift/assisted-service/restapi/operations/installer"
operations "github.com/openshift/assisted-service/restapi/operations/manifests"
hivev1 "github.com/openshift/hive/apis/hive/v1"
imageReference "github.com/openshift/library-go/pkg/image/reference"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/thoas/go-funk"
Expand Down Expand Up @@ -104,9 +106,10 @@ type ClusterDeploymentsReconciler struct {
Manifests manifestsapi.ClusterManifestsInternals
ServiceBaseURL string
PullSecretHandler
AuthType auth.AuthType
VersionsHandler versions.Handler
SpokeK8sClientFactory spoke_k8s_client.SpokeK8sClientFactory
AuthType auth.AuthType
VersionsHandler versions.Handler
SpokeK8sClientFactory spoke_k8s_client.SpokeK8sClientFactory
MirrorRegistriesConfigBuilder mirrorregistries.MirrorRegistriesConfigBuilder
}

const minimalOpenShiftVersionForDefaultNetworkTypeOVNKubernetes = "4.12.0-0.0"
Expand Down Expand Up @@ -199,7 +202,7 @@ func (r *ClusterDeploymentsReconciler) Reconcile(origCtx context.Context, req ct
}

cluster, err := r.Installer.GetClusterByKubeKey(req.NamespacedName)
if err1 := r.validateClusterDeployment(clusterDeployment, clusterInstall); err1 != nil {
if err1 := r.validateClusterDeployment(ctx, clusterDeployment, clusterInstall); err1 != nil {
log.Error(err1)
return r.updateStatus(ctx, log, clusterInstall, clusterDeployment, cluster, err1)
}
Expand Down Expand Up @@ -253,14 +256,30 @@ func (r *ClusterDeploymentsReconciler) Reconcile(origCtx context.Context, req ct
return r.updateStatus(ctx, log, clusterInstall, clusterDeployment, cluster, nil)
}

func (r *ClusterDeploymentsReconciler) validateClusterDeployment(clusterDeployment *hivev1.ClusterDeployment,
func (r *ClusterDeploymentsReconciler) validateClusterDeployment(ctx context.Context, clusterDeployment *hivev1.ClusterDeployment,
clusterInstall *hiveext.AgentClusterInstall) error {

// Make sure that the ImageSetRef is set for clusters not already installed
if clusterInstall.Spec.ImageSetRef == nil && !clusterDeployment.Spec.Installed {
return newInputError("missing ImageSetRef for cluster that is not installed")
}

// If mirror registries are enabled then we need to make sure that the release image is not tag based.
// A tagged release image will be rejected by oc when mirror registry configuration is present.
// Mirrored images are typically used in disconnected environments which can have trouble with tag based images.
if clusterInstall.Spec.ImageSetRef != nil && r.MirrorRegistriesConfigBuilder.IsMirrorRegistriesConfigured() {
releaseImageURL, err := r.getReleaseImageURL(ctx, clusterInstall.Spec)
if err != nil {
return errors.Wrapf(err, "unable to fetch release image url")
}
ref, err := imageReference.Parse(releaseImageURL)
if err != nil {
return newInputError(fmt.Sprintf("unable to parse release image : %s due to error %s, cannot check for mirror registry compliance", releaseImageURL, err.Error()))
}
if len(ref.Tag) > 0 {
return newInputError(fmt.Sprintf("the supplied release image %s cannot be used with a mirror registry configuration, the image is tag based and should instead be based on a digest", releaseImageURL))
}
}
return nil
}

Expand Down Expand Up @@ -1436,6 +1455,21 @@ func (r *ClusterDeploymentsReconciler) createNewDay2Cluster(
return r.updateStatus(ctx, log, clusterInstall, clusterDeployment, c, err)
}

func (r *ClusterDeploymentsReconciler) getReleaseImageURL(
ctx context.Context,
spec hiveext.AgentClusterInstallSpec) (string, error) {

clusterImageSet := &hivev1.ClusterImageSet{}
key := types.NamespacedName{
Namespace: "",
Name: spec.ImageSetRef.Name,
}
if err := r.Client.Get(ctx, key, clusterImageSet); err != nil {
return "", errors.Wrapf(err, "failed to get cluster image set %s", key.Name)
}
return clusterImageSet.Spec.ReleaseImage, nil
}

func (r *ClusterDeploymentsReconciler) getReleaseImage(
ctx context.Context,
log logrus.FieldLogger,
Expand All @@ -1451,11 +1485,16 @@ func (r *ClusterDeploymentsReconciler) getReleaseImage(
return nil, errors.Wrapf(err, "failed to get cluster image set %s", key.Name)
}

releaseImage, err := r.VersionsHandler.GetReleaseImageByURL(ctx, clusterImageSet.Spec.ReleaseImage, pullSecret)
releaseImageUrl, err := r.getReleaseImageURL(ctx, spec)
if err != nil {
return nil, errors.Wrapf(err, "unable to fetch release image url")
}

releaseImage, err := r.VersionsHandler.GetReleaseImageByURL(ctx, releaseImageUrl, pullSecret)
if err != nil {
log.Error(err)
errMsg := "failed to get release image '%s'. Please ensure the releaseImage field in ClusterImageSet '%s' is valid (error: %s)."
return nil, errors.New(fmt.Sprintf(errMsg, clusterImageSet.Spec.ReleaseImage, spec.ImageSetRef.Name, err.Error()))
return nil, errors.New(fmt.Sprintf(errMsg, releaseImageUrl, spec.ImageSetRef.Name, err.Error()))
}

return releaseImage, nil
Expand Down
Loading

0 comments on commit bc33455

Please sign in to comment.