From 439375ea29849ea2fc0302c8c92b049859e09cfc Mon Sep 17 00:00:00 2001 From: Matthew Barnes Date: Thu, 18 Jan 2024 15:20:03 -0500 Subject: [PATCH] Move default openshift version (#3094) * api: Avoid referencing DefaultInstallStream in tests * frontend: Avoid referencing DefaultInstallStream The frontend's OpenShiftVersions change feed handler will record the current default version for the rest of the frontend to use. * monitor: Remove latestGaMinorVersion metric The RP no longer has this information internally, so the metric is no longer relevant. * update_ocp_versions: Read versions from an environment variable Read OpenShift versions and pull specs from an OPENSHIFT_VERSIONS environment variable containing a JSON object. This data includes the default OpenShift version for new installs that don't specify a version. This moves us toward eliminating hard-coded OpenShift versions in pkg/util/version/const.go. * cache_fallback_discovery_client_test.go: Hard-code version I'm not sure what to do with this test. Install stream data has moved to RP-Config, so if the test is worth keeping then I guess the oldest supported version will have to be hard-coded and kept up-to-date. But it probably won't be. * version: Remove DefaultInstallStreams DefaultInstallStream will remain for now, but it's ONLY for use by local development mode until we can come up with a better solution. --------- Co-authored-by: Matthew Barnes --- cmd/aro/const.go | 10 +- cmd/aro/dbtoken.go | 8 +- cmd/aro/gateway.go | 8 +- cmd/aro/main.go | 4 +- cmd/aro/monitor.go | 8 +- cmd/aro/portal.go | 8 +- cmd/aro/rp.go | 4 +- cmd/aro/update_ocp_versions.go | 111 ++++++++++++++---- .../openshiftcluster_validatestatic_test.go | 3 +- .../openshiftcluster_validatestatic_test.go | 3 +- .../openshiftcluster_validatestatic_test.go | 3 +- .../openshiftcluster_validatestatic_test.go | 3 +- pkg/frontend/admin_openshiftversion_put.go | 12 +- .../admin_openshiftversion_put_test.go | 9 +- pkg/frontend/changefeed.go | 3 + pkg/frontend/frontend.go | 1 + ...enshiftcluster_preflightvalidation_test.go | 6 +- .../openshiftcluster_putorpatch_test.go | 12 +- pkg/frontend/openshiftversions_list.go | 9 -- pkg/frontend/openshiftversions_list_test.go | 15 ++- pkg/frontend/validate.go | 11 +- pkg/monitor/cluster/clusterversions.go | 11 +- pkg/monitor/cluster/clusterversions_test.go | 2 - .../cache_fallback_discovery_client_test.go | 3 +- pkg/util/version/const.go | 32 +---- test/e2e/setup.go | 5 + 26 files changed, 172 insertions(+), 132 deletions(-) diff --git a/cmd/aro/const.go b/cmd/aro/const.go index 358dc6b2021..65d924c316d 100644 --- a/cmd/aro/const.go +++ b/cmd/aro/const.go @@ -4,8 +4,10 @@ package main // Licensed under the Apache License 2.0. const ( - DatabaseName = "DATABASE_NAME" - DatabaseAccountName = "DATABASE_ACCOUNT_NAME" - KeyVaultPrefix = "KEYVAULT_PREFIX" - DBTokenUrl = "DBTOKEN_URL" + envDatabaseName = "DATABASE_NAME" + envDatabaseAccountName = "DATABASE_ACCOUNT_NAME" + envKeyVaultPrefix = "KEYVAULT_PREFIX" + envDBTokenUrl = "DBTOKEN_URL" + envOpenShiftVersions = "OPENSHIFT_VERSIONS" + envInstallerImageDigests = "INSTALLER_IMAGE_DIGESTS" ) diff --git a/cmd/aro/dbtoken.go b/cmd/aro/dbtoken.go index 40b76a77a5e..f905d35a075 100644 --- a/cmd/aro/dbtoken.go +++ b/cmd/aro/dbtoken.go @@ -56,11 +56,11 @@ func dbtoken(ctx context.Context, log *logrus.Entry) error { go g.Run() - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return err } - dbAccountName := os.Getenv(DatabaseAccountName) + dbAccountName := os.Getenv(envDatabaseAccountName) clientOptions := &policy.ClientOptions{ ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions, @@ -87,10 +87,10 @@ func dbtoken(ctx context.Context, log *logrus.Entry) error { return err } - if err := env.ValidateVars(KeyVaultPrefix); err != nil { + if err := env.ValidateVars(envKeyVaultPrefix); err != nil { return err } - keyVaultPrefix := os.Getenv(KeyVaultPrefix) + keyVaultPrefix := os.Getenv(envKeyVaultPrefix) dbtokenKeyvaultURI := keyvault.URI(_env, env.DBTokenKeyvaultSuffix, keyVaultPrefix) dbtokenKeyvault := keyvault.NewManager(msiKVAuthorizer, dbtokenKeyvaultURI) diff --git a/cmd/aro/gateway.go b/cmd/aro/gateway.go index 3262a7e139c..49956b65b25 100644 --- a/cmd/aro/gateway.go +++ b/cmd/aro/gateway.go @@ -41,10 +41,10 @@ func gateway(ctx context.Context, log *logrus.Entry) error { go g.Run() - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return err } - dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, nil, m, nil, os.Getenv(DatabaseAccountName)) + dbc, err := database.NewDatabaseClient(log.WithField("component", "database"), _env, nil, m, nil, os.Getenv(envDatabaseAccountName)) if err != nil { return err } @@ -136,9 +136,9 @@ func getURL(isLocalDevelopmentMode bool) (string, error) { return "https://localhost:8445", nil } - if err := env.ValidateVars(DBTokenUrl); err != nil { + if err := env.ValidateVars(envDBTokenUrl); err != nil { return "", err } - return os.Getenv(DBTokenUrl), nil + return os.Getenv(envDBTokenUrl), nil } diff --git a/cmd/aro/main.go b/cmd/aro/main.go index 6a7ece861b8..80b34eb0529 100644 --- a/cmd/aro/main.go +++ b/cmd/aro/main.go @@ -108,9 +108,9 @@ func DBName(isLocalDevelopmentMode bool) (string, error) { return "ARO", nil } - if err := env.ValidateVars(DatabaseName); err != nil { + if err := env.ValidateVars(envDatabaseName); err != nil { return "", fmt.Errorf("%v (development mode)", err.Error()) } - return os.Getenv(DatabaseName), nil + return os.Getenv(envDatabaseName), nil } diff --git a/cmd/aro/monitor.go b/cmd/aro/monitor.go index 18ae8e368ff..307a2aecf0d 100644 --- a/cmd/aro/monitor.go +++ b/cmd/aro/monitor.go @@ -70,10 +70,10 @@ func monitor(ctx context.Context, log *logrus.Entry) error { return err } - if err := env.ValidateVars(KeyVaultPrefix); err != nil { + if err := env.ValidateVars(envKeyVaultPrefix); err != nil { return err } - keyVaultPrefix := os.Getenv(KeyVaultPrefix) + keyVaultPrefix := os.Getenv(envKeyVaultPrefix) // TODO: should not be using the service keyvault here serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix) serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI) @@ -83,11 +83,11 @@ func monitor(ctx context.Context, log *logrus.Entry) error { return err } - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return err } - dbAccountName := os.Getenv(DatabaseAccountName) + dbAccountName := os.Getenv(envDatabaseAccountName) clientOptions := &policy.ClientOptions{ ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions, } diff --git a/cmd/aro/portal.go b/cmd/aro/portal.go index 94d51d4aa7d..979100bdf47 100644 --- a/cmd/aro/portal.go +++ b/cmd/aro/portal.go @@ -80,10 +80,10 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error { go g.Run() - if err := env.ValidateVars(KeyVaultPrefix); err != nil { + if err := env.ValidateVars(envKeyVaultPrefix); err != nil { return err } - keyVaultPrefix := os.Getenv(KeyVaultPrefix) + keyVaultPrefix := os.Getenv(envKeyVaultPrefix) // TODO: should not be using the service keyvault here serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix) serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI) @@ -93,11 +93,11 @@ func portal(ctx context.Context, log *logrus.Entry, audit *logrus.Entry) error { return err } - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return err } - dbAccountName := os.Getenv(DatabaseAccountName) + dbAccountName := os.Getenv(envDatabaseAccountName) clientOptions := &policy.ClientOptions{ ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions, } diff --git a/cmd/aro/rp.go b/cmd/aro/rp.go index 51feb0a4bc1..e319e407140 100644 --- a/cmd/aro/rp.go +++ b/cmd/aro/rp.go @@ -103,11 +103,11 @@ func rp(ctx context.Context, log, audit *logrus.Entry) error { return err } - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return err } - dbAccountName := os.Getenv(DatabaseAccountName) + dbAccountName := os.Getenv(envDatabaseAccountName) clientOptions := &policy.ClientOptions{ ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions, } diff --git a/cmd/aro/update_ocp_versions.go b/cmd/aro/update_ocp_versions.go index 1ffb64af686..e7c128c6f52 100644 --- a/cmd/aro/update_ocp_versions.go +++ b/cmd/aro/update_ocp_versions.go @@ -22,8 +22,13 @@ import ( "github.com/Azure/ARO-RP/pkg/util/version" ) -func getInstallerImageDigests(envKey string) (map[string]string, error) { - var installerImageDigests map[string]string +// Corresponds to configuration.openShiftVersions in RP-Config +type OpenShiftVersions struct { + DefaultStream map[string]string + InstallStreams map[string]string +} + +func getEnvironmentData(envKey string, envData any) error { var err error jsonData := []byte(os.Getenv(envKey)) @@ -34,50 +39,70 @@ func getInstallerImageDigests(envKey string) (map[string]string, error) { if !env.IsLocalDevelopmentMode() { jsonData, err = base64.StdEncoding.DecodeString(string(jsonData)) if err != nil { - return nil, fmt.Errorf("%s: Failed to decode base64: %v", envKey, err) + return fmt.Errorf("%s: Failed to decode base64: %w", envKey, err) } } - if err = json.Unmarshal(jsonData, &installerImageDigests); err != nil { - return nil, fmt.Errorf("%s: Failed to parse JSON: %v", envKey, err) + if err = json.Unmarshal(jsonData, envData); err != nil { + return fmt.Errorf("%s: Failed to parse JSON: %w", envKey, err) } - return installerImageDigests, nil + return nil } -func getLatestOCPVersions(ctx context.Context, log *logrus.Entry) ([]api.OpenShiftVersion, error) { - env, err := env.NewCoreForCI(ctx, log) - if err != nil { +func getOpenShiftVersions() (*OpenShiftVersions, error) { + const envKey = envOpenShiftVersions + var openShiftVersions OpenShiftVersions + + if err := getEnvironmentData(envKey, &openShiftVersions); err != nil { return nil, err } - dstAcr := os.Getenv("DST_ACR_NAME") - acrDomainSuffix := "." + env.Environment().ContainerRegistryDNSSuffix - dstRepo := dstAcr + acrDomainSuffix - ocpVersions := []api.OpenShiftVersion{} + // The DefaultStream map must have exactly one entry. + numDefaultStreams := len(openShiftVersions.DefaultStream) + if numDefaultStreams != 1 { + return nil, fmt.Errorf("%s: DefaultStream must have exactly 1 entry, found %d", envKey, numDefaultStreams) + } + + return &openShiftVersions, nil +} +func getInstallerImageDigests() (map[string]string, error) { // INSTALLER_IMAGE_DIGESTS is the mapping of a minor version to // the aro-installer wrapper digest. This allows us to utilize // Azure Safe Deployment Practices (SDP) instead of pushing the // version tag and deploying to all regions at once. - installerImageDigests, err := getInstallerImageDigests("INSTALLER_IMAGE_DIGESTS") - if err != nil { + const envKey = envInstallerImageDigests + var installerImageDigests map[string]string + + if err := getEnvironmentData(envKey, &installerImageDigests); err != nil { return nil, err } - for _, vers := range version.AvailableInstallStreams { - installerPullSpec := fmt.Sprintf("%s/aro-installer:%s", dstRepo, vers.Version.MinorVersion()) - digest, ok := installerImageDigests[vers.Version.MinorVersion()] + return installerImageDigests, nil +} + +func appendOpenShiftVersions(ocpVersions []api.OpenShiftVersion, installStreams map[string]string, installerImageName string, installerImageDigests map[string]string, isDefault bool) ([]api.OpenShiftVersion, error) { + for fullVersion, openShiftPullspec := range installStreams { + openShiftVersion, err := version.ParseVersion(fullVersion) + if err != nil { + return nil, err + } + fullVersion = openShiftVersion.String() // trimmed of whitespace + minorVersion := openShiftVersion.MinorVersion() + installerDigest, ok := installerImageDigests[minorVersion] if !ok { - return nil, fmt.Errorf("no digest found for version %s", vers.Version.String()) + return nil, fmt.Errorf("no installer digest for version %s", minorVersion) } + installerPullspec := fmt.Sprintf("%s:%s@%s", installerImageName, minorVersion, installerDigest) ocpVersions = append(ocpVersions, api.OpenShiftVersion{ Properties: api.OpenShiftVersionProperties{ - Version: vers.Version.String(), - OpenShiftPullspec: vers.PullSpec, - InstallerPullspec: installerPullSpec + "@" + digest, + Version: fullVersion, + OpenShiftPullspec: openShiftPullspec, + InstallerPullspec: installerPullspec, Enabled: true, + Default: isDefault, }, }) } @@ -85,6 +110,40 @@ func getLatestOCPVersions(ctx context.Context, log *logrus.Entry) ([]api.OpenShi return ocpVersions, nil } +func getLatestOCPVersions(ctx context.Context, log *logrus.Entry) ([]api.OpenShiftVersion, error) { + env, err := env.NewCoreForCI(ctx, log) + if err != nil { + return nil, err + } + dstAcr := os.Getenv("DST_ACR_NAME") + acrDomainSuffix := "." + env.Environment().ContainerRegistryDNSSuffix + installerImageName := dstAcr + acrDomainSuffix + "/aro-installer" + + openShiftVersions, err := getOpenShiftVersions() + if err != nil { + return nil, err + } + + installerImageDigests, err := getInstallerImageDigests() + if err != nil { + return nil, err + } + + ocpVersions := make([]api.OpenShiftVersion, 0, len(openShiftVersions.DefaultStream)+len(openShiftVersions.InstallStreams)) + + ocpVersions, err = appendOpenShiftVersions(ocpVersions, openShiftVersions.DefaultStream, installerImageName, installerImageDigests, true) + if err != nil { + return nil, err + } + + ocpVersions, err = appendOpenShiftVersions(ocpVersions, openShiftVersions.InstallStreams, installerImageName, installerImageDigests, false) + if err != nil { + return nil, err + } + + return ocpVersions, nil +} + func getVersionsDatabase(ctx context.Context, log *logrus.Entry) (database.OpenShiftVersions, error) { _env, err := env.NewCore(ctx, log, env.COMPONENT_UPDATE_OCP_VERSIONS) if err != nil { @@ -113,10 +172,10 @@ func getVersionsDatabase(ctx context.Context, log *logrus.Entry) (database.OpenS m := statsd.New(ctx, log.WithField("component", "update-ocp-versions"), _env, os.Getenv("MDM_ACCOUNT"), os.Getenv("MDM_NAMESPACE"), os.Getenv("MDM_STATSD_SOCKET")) - if err := env.ValidateVars(KeyVaultPrefix); err != nil { + if err := env.ValidateVars(envKeyVaultPrefix); err != nil { return nil, err } - keyVaultPrefix := os.Getenv(KeyVaultPrefix) + keyVaultPrefix := os.Getenv(envKeyVaultPrefix) serviceKeyvaultURI := keyvault.URI(_env, env.ServiceKeyvaultSuffix, keyVaultPrefix) serviceKeyvault := keyvault.NewManager(msiKVAuthorizer, serviceKeyvaultURI) @@ -125,11 +184,11 @@ func getVersionsDatabase(ctx context.Context, log *logrus.Entry) (database.OpenS return nil, err } - if err := env.ValidateVars(DatabaseAccountName); err != nil { + if err := env.ValidateVars(envDatabaseAccountName); err != nil { return nil, err } - dbAccountName := os.Getenv(DatabaseAccountName) + dbAccountName := os.Getenv(envDatabaseAccountName) clientOptions := &policy.ClientOptions{ ClientOptions: _env.Environment().ManagedIdentityCredentialOptions().ClientOptions, } diff --git a/pkg/api/v20191231preview/openshiftcluster_validatestatic_test.go b/pkg/api/v20191231preview/openshiftcluster_validatestatic_test.go index 1023ee267d6..85ce8657466 100644 --- a/pkg/api/v20191231preview/openshiftcluster_validatestatic_test.go +++ b/pkg/api/v20191231preview/openshiftcluster_validatestatic_test.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/util/uuid" - "github.com/Azure/ARO-RP/pkg/util/version" "github.com/Azure/ARO-RP/test/validate" ) @@ -51,7 +50,7 @@ func validOpenShiftCluster() *OpenShiftCluster { ClusterProfile: ClusterProfile{ PullSecret: `{"auths":{"registry.connect.redhat.com":{"auth":""},"registry.redhat.io":{"auth":""}}}`, Domain: "cluster.location.aroapp.io", - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", subscriptionID), }, ConsoleProfile: ConsoleProfile{ diff --git a/pkg/api/v20200430/openshiftcluster_validatestatic_test.go b/pkg/api/v20200430/openshiftcluster_validatestatic_test.go index 7a4c5dd6d94..9ae089e4098 100644 --- a/pkg/api/v20200430/openshiftcluster_validatestatic_test.go +++ b/pkg/api/v20200430/openshiftcluster_validatestatic_test.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/util/uuid" - "github.com/Azure/ARO-RP/pkg/util/version" "github.com/Azure/ARO-RP/test/validate" ) @@ -51,7 +50,7 @@ func validOpenShiftCluster() *OpenShiftCluster { ClusterProfile: ClusterProfile{ PullSecret: `{"auths":{"registry.connect.redhat.com":{"auth":""},"registry.redhat.io":{"auth":""}}}`, Domain: "cluster.location.aroapp.io", - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", subscriptionID), }, ConsoleProfile: ConsoleProfile{ diff --git a/pkg/api/v20210901preview/openshiftcluster_validatestatic_test.go b/pkg/api/v20210901preview/openshiftcluster_validatestatic_test.go index 256c91cbad9..e1614ed13c3 100644 --- a/pkg/api/v20210901preview/openshiftcluster_validatestatic_test.go +++ b/pkg/api/v20210901preview/openshiftcluster_validatestatic_test.go @@ -14,7 +14,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/util/uuid" - "github.com/Azure/ARO-RP/pkg/util/version" "github.com/Azure/ARO-RP/test/validate" ) @@ -65,7 +64,7 @@ func validOpenShiftCluster() *OpenShiftCluster { ClusterProfile: ClusterProfile{ PullSecret: `{"auths":{"registry.connect.redhat.com":{"auth":""},"registry.redhat.io":{"auth":""}}}`, Domain: "cluster.location.aroapp.io", - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", subscriptionID), }, ConsoleProfile: ConsoleProfile{ diff --git a/pkg/api/v20230904/openshiftcluster_validatestatic_test.go b/pkg/api/v20230904/openshiftcluster_validatestatic_test.go index 35a9fca63dd..b0852939bd0 100644 --- a/pkg/api/v20230904/openshiftcluster_validatestatic_test.go +++ b/pkg/api/v20230904/openshiftcluster_validatestatic_test.go @@ -15,7 +15,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/util/uuid" - "github.com/Azure/ARO-RP/pkg/util/version" "github.com/Azure/ARO-RP/test/validate" ) @@ -71,7 +70,7 @@ func validOpenShiftCluster(name, location string) *OpenShiftCluster { ClusterProfile: ClusterProfile{ PullSecret: `{"auths":{"registry.connect.redhat.com":{"auth":""},"registry.redhat.io":{"auth":""}}}`, Domain: "cluster.location.aroapp.io", - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/test-cluster", subscriptionID), FipsValidatedModules: FipsValidatedModulesDisabled, }, diff --git a/pkg/frontend/admin_openshiftversion_put.go b/pkg/frontend/admin_openshiftversion_put.go index 35d9fcd00de..60283d94749 100644 --- a/pkg/frontend/admin_openshiftversion_put.go +++ b/pkg/frontend/admin_openshiftversion_put.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/api/admin" "github.com/Azure/ARO-RP/pkg/frontend/middleware" - "github.com/Azure/ARO-RP/pkg/util/version" ) func (f *frontend) putAdminOpenShiftVersion(w http.ResponseWriter, r *http.Request) { @@ -37,12 +36,6 @@ func (f *frontend) putAdminOpenShiftVersion(w http.ResponseWriter, r *http.Reque return } - // prevent disabling of the default installation version - if ext.Properties.Version == version.DefaultInstallStream.Version.String() && !ext.Properties.Enabled { - api.WriteError(w, http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.enabled", "You cannot disable the default installation version.") - return - } - docs, err := f.dbOpenShiftVersions.ListAll(ctx) if err != nil { api.WriteError(w, http.StatusInternalServerError, api.CloudErrorCodeInternalServerError, "", "Internal server error.") @@ -53,6 +46,11 @@ func (f *frontend) putAdminOpenShiftVersion(w http.ResponseWriter, r *http.Reque if docs != nil { for _, doc := range docs.OpenShiftVersionDocuments { if doc.OpenShiftVersion.Properties.Version == ext.Properties.Version { + // prevent disabling of the default installation version + if doc.OpenShiftVersion.Properties.Default && !ext.Properties.Enabled { + api.WriteError(w, http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.enabled", "You cannot disable the default installation version.") + return + } versionDoc = doc break } diff --git a/pkg/frontend/admin_openshiftversion_put_test.go b/pkg/frontend/admin_openshiftversion_put_test.go index 2f04af784d0..e92e10cb909 100644 --- a/pkg/frontend/admin_openshiftversion_put_test.go +++ b/pkg/frontend/admin_openshiftversion_put_test.go @@ -11,7 +11,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/api/admin" "github.com/Azure/ARO-RP/pkg/metrics/noop" - "github.com/Azure/ARO-RP/pkg/util/version" testdatabase "github.com/Azure/ARO-RP/test/database" ) @@ -224,8 +223,9 @@ func TestOpenShiftVersionPut(t *testing.T) { &api.OpenShiftVersionDocument{ OpenShiftVersion: &api.OpenShiftVersion{ Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", Enabled: true, + Default: true, OpenShiftPullspec: "a:a/b", }, }, @@ -234,7 +234,7 @@ func TestOpenShiftVersionPut(t *testing.T) { }, body: &admin.OpenShiftVersion{ Properties: admin.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", Enabled: false, OpenShiftPullspec: "c:c/d", InstallerPullspec: "d:d/e", @@ -247,8 +247,9 @@ func TestOpenShiftVersionPut(t *testing.T) { ID: "07070707-0707-0707-0707-070707070001", OpenShiftVersion: &api.OpenShiftVersion{ Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), + Version: "4.10.0", Enabled: true, + Default: true, OpenShiftPullspec: "a:a/b", }, }, diff --git a/pkg/frontend/changefeed.go b/pkg/frontend/changefeed.go index 48bb4c07b72..4291e1c9aa0 100644 --- a/pkg/frontend/changefeed.go +++ b/pkg/frontend/changefeed.go @@ -69,6 +69,9 @@ func (f *frontend) updateOcpVersions(docs []*api.OpenShiftVersionDocument) { delete(f.enabledOcpVersions, doc.OpenShiftVersion.Properties.Version) } else { f.enabledOcpVersions[doc.OpenShiftVersion.Properties.Version] = doc.OpenShiftVersion + if doc.OpenShiftVersion.Properties.Default { + f.defaultOcpVersion = doc.OpenShiftVersion.Properties.Version + } } } } diff --git a/pkg/frontend/frontend.go b/pkg/frontend/frontend.go index f7e366eff53..37e80f87c36 100644 --- a/pkg/frontend/frontend.go +++ b/pkg/frontend/frontend.go @@ -63,6 +63,7 @@ type frontend struct { dbSubscriptions database.Subscriptions dbOpenShiftVersions database.OpenShiftVersions + defaultOcpVersion string // always enabled enabledOcpVersions map[string]*api.OpenShiftVersion apis map[string]*api.Version diff --git a/pkg/frontend/openshiftcluster_preflightvalidation_test.go b/pkg/frontend/openshiftcluster_preflightvalidation_test.go index a60a1576b76..fe70615c31c 100644 --- a/pkg/frontend/openshiftcluster_preflightvalidation_test.go +++ b/pkg/frontend/openshiftcluster_preflightvalidation_test.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/metrics/noop" - "github.com/Azure/ARO-RP/pkg/util/version" testdatabase "github.com/Azure/ARO-RP/test/database" ) @@ -167,10 +166,11 @@ func TestPreflightValidation(t *testing.T) { go f.Run(ctx, nil, nil) f.mu.Lock() + f.defaultOcpVersion = "4.10.0" f.enabledOcpVersions = map[string]*api.OpenShiftVersion{ - version.DefaultInstallStream.Version.String(): { + f.defaultOcpVersion: { Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), + Version: f.defaultOcpVersion, }, }, } diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index c3978cb12f0..afb59df38b7 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -1796,7 +1796,7 @@ func TestPutOrPatchOpenShiftClusterAdminAPI(t *testing.T) { func TestPutOrPatchOpenShiftCluster(t *testing.T) { ctx := context.Background() - defaultVersion := version.DefaultInstallStream.Version.String() + defaultVersion := "4.10.0" apis := map[string]*api.Version{ "2020-04-30": { @@ -1807,10 +1807,11 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { } defaultVersionChangeFeed := map[string]*api.OpenShiftVersion{ - version.DefaultInstallStream.Version.String(): { + defaultVersion: { Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), + Version: defaultVersion, Enabled: true, + Default: true, }, }, } @@ -2813,6 +2814,11 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { go f.Run(ctx, nil, nil) f.mu.Lock() f.enabledOcpVersions = tt.changeFeed + for key, doc := range tt.changeFeed { + if doc.Properties.Default { + f.defaultOcpVersion = key + } + } f.mu.Unlock() oc := &v20200430.OpenShiftCluster{} diff --git a/pkg/frontend/openshiftversions_list.go b/pkg/frontend/openshiftversions_list.go index c6bdb9f0675..c3de112b302 100644 --- a/pkg/frontend/openshiftversions_list.go +++ b/pkg/frontend/openshiftversions_list.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/frontend/middleware" - "github.com/Azure/ARO-RP/pkg/util/version" ) func (f *frontend) listInstallVersions(w http.ResponseWriter, r *http.Request) { @@ -42,13 +41,5 @@ func (f *frontend) getEnabledInstallVersions(ctx context.Context) []*api.OpenShi } f.mu.RUnlock() - if len(versions) == 0 { - versions = append(versions, &api.OpenShiftVersion{ - Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStream.Version.String(), - }, - }) - } - return versions } diff --git a/pkg/frontend/openshiftversions_list_test.go b/pkg/frontend/openshiftversions_list_test.go index a776ab601f6..3f1413c9003 100644 --- a/pkg/frontend/openshiftversions_list_test.go +++ b/pkg/frontend/openshiftversions_list_test.go @@ -16,7 +16,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" v20220904 "github.com/Azure/ARO-RP/pkg/api/v20220904" "github.com/Azure/ARO-RP/pkg/metrics/noop" - "github.com/Azure/ARO-RP/pkg/util/version" ) func TestListInstallVersions(t *testing.T) { @@ -37,9 +36,9 @@ func TestListInstallVersions(t *testing.T) { { name: "return multiple versions", changeFeed: map[string]*api.OpenShiftVersion{ - version.DefaultInstallStreams[11].Version.String(): { + "4.11.0": { Properties: api.OpenShiftVersionProperties{ - Version: version.DefaultInstallStreams[11].Version.String(), + Version: "4.11.0", Enabled: true, }, }, @@ -47,6 +46,7 @@ func TestListInstallVersions(t *testing.T) { Properties: api.OpenShiftVersionProperties{ Version: "4.11.5", Enabled: true, + Default: true, }, }, }, @@ -56,12 +56,12 @@ func TestListInstallVersions(t *testing.T) { OpenShiftVersions: []*v20220904.OpenShiftVersion{ { Properties: v20220904.OpenShiftVersionProperties{ - Version: "4.11.5", + Version: "4.11.0", }, }, { Properties: v20220904.OpenShiftVersionProperties{ - Version: version.DefaultInstallStreams[11].Version.String(), + Version: "4.11.5", }, }, }, @@ -87,6 +87,11 @@ func TestListInstallVersions(t *testing.T) { frontend.mu.Lock() frontend.enabledOcpVersions = tt.changeFeed + for key, doc := range tt.changeFeed { + if doc.Properties.Enabled { + frontend.defaultOcpVersion = key + } + } frontend.mu.Unlock() resp, b, err := ti.request(method, diff --git a/pkg/frontend/validate.go b/pkg/frontend/validate.go index d30688301bf..7167c1add14 100644 --- a/pkg/frontend/validate.go +++ b/pkg/frontend/validate.go @@ -16,7 +16,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api/validate" "github.com/Azure/ARO-RP/pkg/database/cosmosdb" utilnamespace "github.com/Azure/ARO-RP/pkg/util/namespace" - "github.com/Azure/ARO-RP/pkg/util/version" ) func validateTerminalProvisioningState(state api.ProvisioningState) error { @@ -204,14 +203,12 @@ func validateAdminMasterVMSize(vmSize string) error { // validateInstallVersion validates the install version set in the clusterprofile.version // TODO convert this into static validation instead of this receiver function in the validation for frontend. func (f *frontend) validateInstallVersion(ctx context.Context, oc *api.OpenShiftCluster) error { - // If this request is from an older API or the user never specified - // the version to install we default to the DefaultInstallStream.Version - // TODO: We should set default version in cosmosdb instead of hardcoding it in golang code + f.mu.RLock() + // If this request is from an older API or the user did not specify + // the version to install, use the default version. if oc.Properties.ClusterProfile.Version == "" { - oc.Properties.ClusterProfile.Version = version.DefaultInstallStream.Version.String() + oc.Properties.ClusterProfile.Version = f.defaultOcpVersion } - - f.mu.RLock() _, ok := f.enabledOcpVersions[oc.Properties.ClusterProfile.Version] f.mu.RUnlock() diff --git a/pkg/monitor/cluster/clusterversions.go b/pkg/monitor/cluster/clusterversions.go index 86231df69cb..c42c8aeb5c6 100644 --- a/pkg/monitor/cluster/clusterversions.go +++ b/pkg/monitor/cluster/clusterversions.go @@ -51,12 +51,11 @@ func (mon *Monitor) emitClusterVersions(ctx context.Context) error { mon.emitGauge("cluster.versions", 1, map[string]string{ "actualVersion": actualVersion, "desiredVersion": desiredVersion(cv), - "provisionedByResourceProviderVersion": mon.oc.Properties.ProvisionedBy, // last successful Put or Patch - "resourceProviderVersion": version.GitCommit, // RP version currently running - "operatorVersion": operatorVersion, // operator version in the cluster - "availableRP": availableRP, // current RP version available for document update, empty when none - "latestGaMinorVersion": version.DefaultInstallStream.Version.MinorVersion(), // Latest GA in ARO Minor version - "actualMinorVersion": actualMinorVersion, // Minor version, empty if actual version is not in expected form + "provisionedByResourceProviderVersion": mon.oc.Properties.ProvisionedBy, // last successful Put or Patch + "resourceProviderVersion": version.GitCommit, // RP version currently running + "operatorVersion": operatorVersion, // operator version in the cluster + "availableRP": availableRP, // current RP version available for document update, empty when none + "actualMinorVersion": actualMinorVersion, // Minor version, empty if actual version is not in expected form }) return nil diff --git a/pkg/monitor/cluster/clusterversions_test.go b/pkg/monitor/cluster/clusterversions_test.go index b96b9ebc37d..6eb2050ad2d 100644 --- a/pkg/monitor/cluster/clusterversions_test.go +++ b/pkg/monitor/cluster/clusterversions_test.go @@ -17,7 +17,6 @@ import ( "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/operator" mock_metrics "github.com/Azure/ARO-RP/pkg/util/mocks/metrics" - "github.com/Azure/ARO-RP/pkg/util/version" ) func TestEmitClusterVersion(t *testing.T) { @@ -158,7 +157,6 @@ func TestEmitClusterVersion(t *testing.T) { "operatorVersion": "test", "resourceProviderVersion": "unknown", "availableRP": tt.wantAvailableRP, - "latestGaMinorVersion": version.DefaultInstallStream.Version.MinorVersion(), "actualMinorVersion": tt.wantActualMinorVersion, }) diff --git a/pkg/util/dynamichelper/discovery/cache_fallback_discovery_client_test.go b/pkg/util/dynamichelper/discovery/cache_fallback_discovery_client_test.go index 949a7b1cca2..f8540f0ae1d 100644 --- a/pkg/util/dynamichelper/discovery/cache_fallback_discovery_client_test.go +++ b/pkg/util/dynamichelper/discovery/cache_fallback_discovery_client_test.go @@ -20,7 +20,6 @@ import ( "github.com/Azure/ARO-RP/pkg/util/cmp" utillog "github.com/Azure/ARO-RP/pkg/util/log" - "github.com/Azure/ARO-RP/pkg/util/version" utilerror "github.com/Azure/ARO-RP/test/util/error" ) @@ -38,7 +37,7 @@ func TestVersion(t *testing.T) { assetsVersion := strings.TrimSuffix(string(b), "\n") // NOTE: This is checking for the version of the oldest supported minor stream. - if assetsVersion != version.DefaultInstallStreams[11].Version.String() { + if assetsVersion != "4.11.44" { t.Error("discovery cache is out of date: run make discoverycache") } } diff --git a/pkg/util/version/const.go b/pkg/util/version/const.go index cb85db39a4b..c9af33c4562 100644 --- a/pkg/util/version/const.go +++ b/pkg/util/version/const.go @@ -30,32 +30,12 @@ type Stream struct { PullSpec string `json:"-"` } -// DefaultMinorVersion describes the minor OpenShift version to default to -var DefaultMinorVersion = 12 - -// DefaultInstallStreams describes the latest version of our supported streams -var DefaultInstallStreams = map[int]*Stream{ - 11: { - Version: NewVersion(4, 11, 44), - PullSpec: "quay.io/openshift-release-dev/ocp-release@sha256:52cbfbbeb9cc03b49c2788ac7333e63d3dae14673e01a9d8e59270f3a8390ed3", - }, - 12: { - Version: NewVersion(4, 12, 25), - PullSpec: "quay.io/openshift-release-dev/ocp-release@sha256:5a4fb052cda1d14d1e306ce87e6b0ded84edddaa76f1cf401bcded99cef2ad84", - }, - 13: { - Version: NewVersion(4, 13, 23), - PullSpec: "quay.io/openshift-release-dev/ocp-release@sha256:ca556d3494d08765c90481f15dd965995371168ea7ee7a551000bed4481931c8", - }, -} - -// DefaultInstallStream describes stream we are defaulting to for all new clusters -var DefaultInstallStream = DefaultInstallStreams[DefaultMinorVersion] - -var AvailableInstallStreams = []*Stream{ - DefaultInstallStreams[11], - DefaultInstallStreams[12], - DefaultInstallStreams[13], +// Install stream data for production and INT has moved to RP-Config. +// This default is left here ONLY for use by local development mode, +// until we can come up with a better solution. +var DefaultInstallStream = Stream{ + Version: NewVersion(4, 12, 25), + PullSpec: "quay.io/openshift-release-dev/ocp-release@sha256:5a4fb052cda1d14d1e306ce87e6b0ded84edddaa76f1cf401bcded99cef2ad84", } // FluentbitImage contains the location of the Fluentbit container image diff --git a/test/e2e/setup.go b/test/e2e/setup.go index 8d4840c4f59..d25bc2d46f5 100644 --- a/test/e2e/setup.go +++ b/test/e2e/setup.go @@ -48,6 +48,7 @@ import ( msgraph_errors "github.com/Azure/ARO-RP/pkg/util/graph/graphsdk/models/odataerrors" utillog "github.com/Azure/ARO-RP/pkg/util/log" "github.com/Azure/ARO-RP/pkg/util/uuid" + "github.com/Azure/ARO-RP/pkg/util/version" "github.com/Azure/ARO-RP/test/util/kubeadminkubeconfig" ) @@ -409,6 +410,10 @@ func setup(ctx context.Context) error { return err } + if osClusterVersion == "" { + osClusterVersion = version.DefaultInstallStream.Version.String() + } + err = cluster.Create(ctx, vnetResourceGroup, clusterName, osClusterVersion) if err != nil { return err