diff --git a/pkg/cli/admin/release/extract.go b/pkg/cli/admin/release/extract.go index 070535643e..b9b37cc892 100644 --- a/pkg/cli/admin/release/extract.go +++ b/pkg/cli/admin/release/extract.go @@ -21,17 +21,20 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericiooptions" + appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" "k8s.io/client-go/rest" kcmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/util/templates" imagev1 "github.com/openshift/api/image/v1" + configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" "github.com/openshift/library-go/pkg/image/dockerv1client" "github.com/openshift/library-go/pkg/manifest" "github.com/openshift/oc/pkg/cli/image/extract" "github.com/openshift/oc/pkg/cli/image/imagesource" imagemanifest "github.com/openshift/oc/pkg/cli/image/manifest" "github.com/openshift/oc/pkg/cli/image/workqueue" + "github.com/openshift/oc/pkg/version" "github.com/pkg/errors" ) @@ -94,7 +97,8 @@ func NewExtract(f kcmdutil.Factory, streams genericiooptions.IOStreams) *cobra.C If --install-config is set, it will be used to determine the expected cluster configuration, otherwise the command will interrogate your current cluster to determine its configuration. This command is most accurate when the version of the extracting client matches the version - of the cluster under consideration. + of the cluster under consideration. Otherwise, for example, newly introduced capabilities in + the version of the extracting client might be considered enabled. Instead of extracting the manifests, you can specify --git=DIR to perform a Git checkout of the source code that comprises the release. A warning will be printed @@ -359,10 +363,27 @@ func (o *ExtractOptions) Run(ctx context.Context) error { if o.Included { context := "connected cluster" inclusionConfig := manifestInclusionConfiguration{} + + clientVersion, reportedVersion, versionErr := version.ExtractVersion() + if versionErr != nil { + return versionErr + } + if reportedVersion == "" { + reportedVersion = clientVersion.String() + } + if o.InstallConfig == "" { - inclusionConfig, err = findClusterIncludeConfig(ctx, o.RESTConfig) + configv1client, configv1clientErr := configv1client.NewForConfig(o.RESTConfig) + if configv1clientErr != nil { + return configv1clientErr + } + appsv1client, appsv1clientErr := appsv1client.NewForConfig(o.RESTConfig) + if appsv1clientErr != nil { + return appsv1clientErr + } + inclusionConfig, err = findClusterIncludeConfig(ctx, configv1client, appsv1client, reportedVersion) } else { - inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig) + inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig, reportedVersion) context = o.InstallConfig } if err != nil { diff --git a/pkg/cli/admin/release/extract_tools.go b/pkg/cli/admin/release/extract_tools.go index bfb46da691..21e44f8e56 100644 --- a/pkg/cli/admin/release/extract_tools.go +++ b/pkg/cli/admin/release/extract_tools.go @@ -14,7 +14,6 @@ import ( "io" "os" "path/filepath" - "regexp" "runtime" "sort" "strings" @@ -32,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/cli-runtime/pkg/genericiooptions" appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" - "k8s.io/client-go/rest" "sigs.k8s.io/yaml" "github.com/MakeNowJust/heredoc" @@ -43,7 +41,6 @@ import ( "github.com/openshift/oc/pkg/cli/admin/internal/codesign" "github.com/openshift/oc/pkg/cli/image/extract" "github.com/openshift/oc/pkg/cli/image/imagesource" - "github.com/openshift/oc/pkg/version" ) // extractTarget describes how a file in the release image can be extracted to disk. @@ -1167,17 +1164,9 @@ func copyAndReplace(errorOutput io.Writer, w io.Writer, r io.Reader, bufferSize } -func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfigPath string) (manifestInclusionConfiguration, error) { +func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfigPath, ocVersion string) (manifestInclusionConfiguration, error) { config := manifestInclusionConfiguration{} - clientVersion, reportedVersion, err := version.ExtractVersion() - if err != nil { - return config, err - } - if reportedVersion == "" { - reportedVersion = clientVersion.String() - } - installConfigBytes, err := os.ReadFile(installConfigPath) if err != nil { return config, err @@ -1205,54 +1194,61 @@ func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfi return config, fmt.Errorf("unrecognized baselineCapabilitySet %q", data.Capabilities.BaselineCapabilitySet) } else { if data.Capabilities.BaselineCapabilitySet == configv1.ClusterVersionCapabilitySetCurrent { - klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the actual %s capability set may differ.", reportedVersion, data.Capabilities.BaselineCapabilitySet) + klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the actual %s capability set may differ.", ocVersion, data.Capabilities.BaselineCapabilitySet) } config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, enabled...) } config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, data.Capabilities.AdditionalEnabledCapabilities...) - klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the known capability sets may differ.", reportedVersion) + klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the known capability sets may differ.", ocVersion) config.Capabilities.KnownCapabilities = configv1.KnownClusterVersionCapabilities } return config, nil } -func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config) (manifestInclusionConfiguration, error) { +func findClusterIncludeConfig(ctx context.Context, configv1client configv1client.ConfigV1Interface, appsv1client appsv1client.AppsV1Interface, ocVersion string) (manifestInclusionConfiguration, error) { config := manifestInclusionConfiguration{} - client, err := configv1client.NewForConfig(restConfig) - if err != nil { - return config, err - } - - if featureGate, err := client.FeatureGates().Get(ctx, "cluster", metav1.GetOptions{}); err != nil { + if featureGate, err := configv1client.FeatureGates().Get(ctx, "cluster", metav1.GetOptions{}); err != nil { return config, err } else { config.RequiredFeatureSet = ptr.To[string](string(featureGate.Spec.FeatureSet)) } - if clusterVersion, err := client.ClusterVersions().Get(ctx, "version", metav1.GetOptions{}); err != nil { + if clusterVersion, err := configv1client.ClusterVersions().Get(ctx, "version", metav1.GetOptions{}); err != nil { return config, err } else { config.Overrides = clusterVersion.Spec.Overrides config.Capabilities = &clusterVersion.Status.Capabilities - // FIXME: eventually pull in GetImplicitlyEnabledCapabilities from https://github.com/openshift/cluster-version-operator/blob/86e24d66119a73f50282b66a8d6f2e3518aa0e15/pkg/payload/payload.go#L237-L240 for cases where a minor update would implicitly enable some additional capabilities. For now, 4.13 to 4.14 will always enable MachineAPI, ImageRegistry, etc.. - currentVersion := clusterVersion.Status.Desired.Version - matches := regexp.MustCompile(`^(\d+[.]\d+)[.].*`).FindStringSubmatch(currentVersion) - if len(matches) < 2 { - return config, fmt.Errorf("failed to parse major.minor version from ClusterVersion status.desired.version %q", currentVersion) - } else if matches[1] == "4.13" { - build := configv1.ClusterVersionCapability("Build") - deploymentConfig := configv1.ClusterVersionCapability("DeploymentConfig") - imageRegistry := configv1.ClusterVersionCapability("ImageRegistry") - config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry) - config.Capabilities.KnownCapabilities = append(config.Capabilities.KnownCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry) + known := sets.New[configv1.ClusterVersionCapability](configv1.KnownClusterVersionCapabilities...) + previouslyKnown := sets.New[configv1.ClusterVersionCapability](config.Capabilities.KnownCapabilities...) + config.Capabilities.KnownCapabilities = previouslyKnown.Union(known).UnsortedList() + + // refresh BaselineCapabilitySet as more capabilities might be included across versions + key := configv1.ClusterVersionCapabilitySetCurrent + if clusterVersion.Spec.Capabilities != nil && clusterVersion.Spec.Capabilities.BaselineCapabilitySet != "" { + key = clusterVersion.Spec.Capabilities.BaselineCapabilitySet } + enabled := sets.New[configv1.ClusterVersionCapability](configv1.ClusterVersionCapabilitySets[key]...) + // The set of the capabilities may grow over time. Without downloading the payload that is running on the cluster, + // it is hard to project all the enabled capabilities after upgrading to the incoming release. + // As an approximation, all newly introduced capabilities are considered enabled to check if a manifest from the + // release should be included while some of them might not be actually enabled on the cluster. + // As a result, unexpected manifests could be included. The number of such manifests is likely small, provided that + // only a small amount of capabilities are added over time and that happens only for minor level updates: + // #Cap(4.11)=4 -> #Cap(4.17)=15, averagely less than two per minor update. + // https://docs.openshift.com/container-platform/4.17/installing/overview/cluster-capabilities.html + deltaKnown := known.Difference(previouslyKnown) + if deltaKnown.Len() > 0 { + klog.Infof("The new capabilities that are introduced in this oc version %s are considered enabled on checking if a manifest is included: %s. They may be disabled on the eventual cluster", ocVersion, deltaKnown.UnsortedList()) + } + enabled = enabled.Union(deltaKnown) + config.Capabilities.EnabledCapabilities = sets.New[configv1.ClusterVersionCapability](config.Capabilities.EnabledCapabilities...).Union(enabled).UnsortedList() } - if infrastructure, err := client.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}); err != nil { + if infrastructure, err := configv1client.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}); err != nil { return config, err } else if infrastructure.Status.PlatformStatus == nil { return config, fmt.Errorf("cluster infrastructure does not declare status.platformStatus: %v", infrastructure.Status) @@ -1260,12 +1256,7 @@ func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config) (man config.Platform = ptr.To[string](strings.ToLower(string(infrastructure.Status.PlatformStatus.Type))) } - appsClient, err := appsv1client.NewForConfig(restConfig) - if err != nil { - return config, err - } - - if deployment, err := appsClient.Deployments("openshift-cluster-version").Get(ctx, "cluster-version-operator", metav1.GetOptions{}); err != nil { + if deployment, err := appsv1client.Deployments("openshift-cluster-version").Get(ctx, "cluster-version-operator", metav1.GetOptions{}); err != nil { return config, err } else { for _, container := range deployment.Spec.Template.Spec.Containers { diff --git a/pkg/cli/admin/release/extract_tools_test.go b/pkg/cli/admin/release/extract_tools_test.go index acffa165b3..7344899096 100644 --- a/pkg/cli/admin/release/extract_tools_test.go +++ b/pkg/cli/admin/release/extract_tools_test.go @@ -2,7 +2,22 @@ package release import ( "bytes" + "context" "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + configv1 "github.com/openshift/api/config/v1" + fakeconfigv1client "github.com/openshift/client-go/config/clientset/versioned/fake" + configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" + + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakekubeclient "k8s.io/client-go/kubernetes/fake" + appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" + "k8s.io/utils/ptr" ) func Test_copyAndReplace(t *testing.T) { @@ -233,3 +248,203 @@ func Test_copyAndReplace(t *testing.T) { }) } } + +func TestFindClusterIncludeConfig(t *testing.T) { + tests := []struct { + name string + configv1client configv1client.ConfigV1Interface + appsv1client appsv1client.AppsV1Interface + expected manifestInclusionConfiguration + expectedErr error + }{ + { + name: "no known disabled capabilities become enabled", + configv1client: fakeconfigv1client.NewClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: "version"}, + Spec: configv1.ClusterVersionSpec{ + Capabilities: &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + }, + }, + Status: configv1.ClusterVersionStatus{ + Capabilities: configv1.ClusterVersionCapabilitiesStatus{ + // no capabilities are enabled + KnownCapabilities: configv1.KnownClusterVersionCapabilities, + }, + }, + }, + &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.FeatureGateSpec{ + FeatureGateSelection: configv1.FeatureGateSelection{ + FeatureSet: configv1.DevPreviewNoUpgrade, + }, + }, + }, + &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{Type: configv1.AWSPlatformType}, + }, + }, + ).ConfigV1(), + appsv1client: fakekubeclient.NewClientset( + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-version-operator", Namespace: "openshift-cluster-version"}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Env: []corev1.EnvVar{{Name: "CLUSTER_PROFILE", Value: "some-profile"}}}}, + }, + }, + }, + }, + ).AppsV1(), + expected: manifestInclusionConfiguration{ + Platform: ptr.To[string]("aws"), + Profile: ptr.To[string]("some-profile"), + RequiredFeatureSet: ptr.To[string](string(configv1.DevPreviewNoUpgrade)), + Capabilities: &configv1.ClusterVersionCapabilitiesStatus{ + KnownCapabilities: configv1.KnownClusterVersionCapabilities, + }, + }, + }, + { + // no known capabilities at all, i.e., all capabilities are new + name: "all new capabilities become enabled", + configv1client: fakeconfigv1client.NewClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: "version"}, + Spec: configv1.ClusterVersionSpec{ + Capabilities: &configv1.ClusterVersionCapabilitiesSpec{ + BaselineCapabilitySet: configv1.ClusterVersionCapabilitySetNone, + }, + }, + }, + &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.FeatureGateSpec{ + FeatureGateSelection: configv1.FeatureGateSelection{ + FeatureSet: configv1.DevPreviewNoUpgrade, + }, + }, + }, + &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{Type: configv1.AWSPlatformType}, + }, + }, + ).ConfigV1(), + appsv1client: fakekubeclient.NewClientset( + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-version-operator", Namespace: "openshift-cluster-version"}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Env: []corev1.EnvVar{{Name: "CLUSTER_PROFILE", Value: "some-profile"}}}}, + }, + }, + }, + }, + ).AppsV1(), + expected: manifestInclusionConfiguration{ + Platform: ptr.To[string]("aws"), + Profile: ptr.To[string]("some-profile"), + RequiredFeatureSet: ptr.To[string](string(configv1.DevPreviewNoUpgrade)), + Capabilities: &configv1.ClusterVersionCapabilitiesStatus{ + EnabledCapabilities: configv1.KnownClusterVersionCapabilities, + KnownCapabilities: configv1.KnownClusterVersionCapabilities, + }, + }, + }, + { + name: "default baseline capabilities become enabled", + configv1client: fakeconfigv1client.NewClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: "version"}, + }, + &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.FeatureGateSpec{ + FeatureGateSelection: configv1.FeatureGateSelection{ + FeatureSet: configv1.DevPreviewNoUpgrade, + }, + }, + }, + &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{Type: configv1.AWSPlatformType}, + }, + }, + ).ConfigV1(), + appsv1client: fakekubeclient.NewClientset( + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster-version-operator", Namespace: "openshift-cluster-version"}, + Spec: appsv1.DeploymentSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Env: []corev1.EnvVar{{Name: "CLUSTER_PROFILE", Value: "some-profile"}}}}, + }, + }, + }, + }, + ).AppsV1(), + expected: manifestInclusionConfiguration{ + Platform: ptr.To[string]("aws"), + Profile: ptr.To[string]("some-profile"), + RequiredFeatureSet: ptr.To[string](string(configv1.DevPreviewNoUpgrade)), + Capabilities: &configv1.ClusterVersionCapabilitiesStatus{ + EnabledCapabilities: configv1.ClusterVersionCapabilitySets[configv1.ClusterVersionCapabilitySetCurrent], + KnownCapabilities: configv1.KnownClusterVersionCapabilities, + }, + }, + }, + { + name: "err on no cvo deployment", + configv1client: fakeconfigv1client.NewClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{Name: "version"}, + }, + &configv1.FeatureGate{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: configv1.FeatureGateSpec{ + FeatureGateSelection: configv1.FeatureGateSelection{ + FeatureSet: configv1.DevPreviewNoUpgrade, + }, + }, + }, + &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + PlatformStatus: &configv1.PlatformStatus{Type: configv1.AWSPlatformType}, + }, + }, + ).ConfigV1(), + appsv1client: fakekubeclient.NewClientset().AppsV1(), + expectedErr: &errors.StatusError{ErrStatus: metav1.Status{Message: `deployments.apps "cluster-version-operator" not found`}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual, actualErr := findClusterIncludeConfig(context.TODO(), tt.configv1client, tt.appsv1client, "x.y.z") + if diff := cmp.Diff(tt.expectedErr, actualErr, cmp.Comparer(func(x, y error) bool { + if x == nil || y == nil { + return x == nil && y == nil + } + return x.Error() == y.Error() + })); diff != "" { + t.Errorf("%s: actualErr differs from expected:\n%s", tt.name, diff) + } + if tt.expectedErr == nil { + // Ignore the diff on nil and []v1.ClusterVersionCapability{} + // Ignore the order on []configv1.ClusterVersionCapability + if diff := cmp.Diff(tt.expected, actual, cmpopts.EquateEmpty(), cmpopts.SortSlices(func(a, b configv1.ClusterVersionCapability) bool { return a < b })); diff != "" { + t.Errorf("%s: actual differs from expected:\n%s", tt.name, diff) + } + } + }) + } +}