Skip to content

Commit 3aacfdd

Browse files
hongkailiuhongkailiu
committed
The ManifestInclusionConfiguration is used to
determine is a manifest is included on a cluster. Its `Capabilities` field takes the implicitly enabled capabilities into account. This change removes the workaround that handles the net-new capabilities introduced by a cluster upgrade. E.g. if a cluster is currently with 4.13, then it assumes that the capabilities "build", "deploymentConfig", and "ImageRegistry" are enabled. This is because the components underlying those capabilities are installed by default on 4.13, or earlier and cannot be disabled once installed. Those capabilities will become enabled after upgrade from 4.13 to 4.14: either explicitly or implicitly depending on the current value of `cv.spec.capabilities.baselineCapabilitySet`. https://github.com/openshift/oc/blob/e005223acd7c478bac070134c16f5533a258be12/pkg/cli/admin/release/extract_tools.go#L1241-L1252 CVO has already defined the function GetImplicitlyEnabledCapabilities that calculates the implicitly enabled capabilities of a cluster after a cluster upgrade. For this function to work, we have to provide * the manifests that are currently included on the cluster. * the manifests from the payload in the upgrade image. The existing `ManifestReceiver` is enhanced in a way that it can provide enabled capabilities, including explicit ones, when the callback to downstream is called. It is implemented by a cache to collect manifests from the upstream and calls downstream only when all manifests are collected and the capabilities are calculated with them using the function `GetImplicitlyEnabledCapabilities` mentioned earlier. This enhancement can be opted in by setting up the `needEnabledCapabilities` field of `ManifestReceiver`. Otherwise, its behaviours stays the same as before. In case that the inclusion configuration is taken from the cluster, i.e., `--install-config` is not set, `needEnabledCapabilities` is set to `true`.
1 parent 289a314 commit 3aacfdd

File tree

3 files changed

+241
-30
lines changed

3 files changed

+241
-30
lines changed

‎pkg/cli/admin/release/extract.go‎

Lines changed: 95 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99
"io"
1010
"os"
11+
"path"
1112
"path/filepath"
1213
"sync"
1314
"time"
@@ -17,6 +18,7 @@ import (
1718
"k8s.io/klog/v2"
1819
"sigs.k8s.io/yaml"
1920

21+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2022
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2123
"k8s.io/apimachinery/pkg/runtime/schema"
2224
"k8s.io/apimachinery/pkg/util/sets"
@@ -25,7 +27,9 @@ import (
2527
kcmdutil "k8s.io/kubectl/pkg/cmd/util"
2628
"k8s.io/kubectl/pkg/util/templates"
2729

30+
configv1 "github.com/openshift/api/config/v1"
2831
imagev1 "github.com/openshift/api/image/v1"
32+
configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
2933
"github.com/openshift/library-go/pkg/image/dockerv1client"
3034
"github.com/openshift/library-go/pkg/manifest"
3135
"github.com/openshift/oc/pkg/cli/image/extract"
@@ -371,17 +375,23 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
371375
versionInImageConfig = v
372376
}
373377

374-
manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata")}
378+
needEnabledCapabilities := o.ExtractManifests && o.Included && o.InstallConfig == ""
379+
var inclusionConfig manifestInclusionConfiguration
380+
manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata"),
381+
needEnabledCapabilities: needEnabledCapabilities}
375382
// o.ExtractManifests implies o.File == ""
376383
if o.ExtractManifests {
377384
expectedProviderSpecKind := credRequestCloudProviderSpecKindMapping[o.Cloud]
378-
379-
include := func(m *manifest.Manifest) error { return nil } // default to including everything
380385
if o.Included {
381386
context := "connected cluster"
382-
inclusionConfig := manifestInclusionConfiguration{}
383387
if o.InstallConfig == "" {
384388
inclusionConfig, err = findClusterIncludeConfig(ctx, o.RESTConfig, versionInImageConfig)
389+
currentPayloadManifests, err := currentPayloadManifests(ctx, opts, o.RESTConfig, inclusionConfig)
390+
if err != nil {
391+
err = fmt.Errorf("failed to get the current payload manifests: %v", err)
392+
} else {
393+
manifestReceiver.currentPayloadManifests = currentPayloadManifests
394+
}
385395
} else {
386396
inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig, versionInImageConfig)
387397
context = o.InstallConfig
@@ -398,10 +408,10 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
398408
return fmt.Errorf("unrecognized platform for CredentialsRequests: %q", *inclusionConfig.Platform)
399409
}
400410
}
401-
include = newIncluder(inclusionConfig)
411+
manifestReceiver.inclusionConfiguration = inclusionConfig
402412
}
403413

404-
manifestReceiver.manifestsCallback = func(filename string, ms []manifest.Manifest, r io.Reader) (bool, error) {
414+
manifestReceiver.manifestsCallback = func(filename string, ms []manifest.Manifest, r io.Reader, enabled []configv1.ClusterVersionCapability) (bool, error) {
405415
if filename == "image-references" && !o.CredentialsRequests {
406416
buf := &bytes.Buffer{}
407417
if _, err := io.Copy(buf, r); err != nil {
@@ -445,12 +455,20 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
445455
return true, nil
446456
}
447457

448-
for i := len(ms) - 1; i >= 0; i-- {
449-
if o.Included && o.CredentialsRequests && ms[i].GVK == credentialsRequestGVK && len(ms[i].Obj.GetAnnotations()) == 0 {
450-
klog.V(4).Infof("Including %s for manual CredentialsRequests, despite lack of annotations", ms[i].String())
451-
} else if err := include(&ms[i]); err != nil {
452-
klog.V(4).Infof("Excluding %s: %s", ms[i].String(), err)
453-
ms = append(ms[:i], ms[i+1:]...)
458+
if o.Included {
459+
for i := len(ms) - 1; i >= 0; i-- {
460+
if o.CredentialsRequests && ms[i].GVK == credentialsRequestGVK && len(ms[i].Obj.GetAnnotations()) == 0 {
461+
klog.V(4).Infof("Including %s for manual CredentialsRequests, despite lack of annotations", ms[i].String())
462+
} else {
463+
clusterVersionCapabilitiesStatus := &configv1.ClusterVersionCapabilitiesStatus{
464+
KnownCapabilities: sets.New[configv1.ClusterVersionCapability](append(inclusionConfig.Capabilities.KnownCapabilities, configv1.KnownClusterVersionCapabilities...)...).UnsortedList(),
465+
EnabledCapabilities: sets.New[configv1.ClusterVersionCapability](append(inclusionConfig.Capabilities.EnabledCapabilities, enabled...)...).UnsortedList(),
466+
}
467+
if err := ms[i].Include(inclusionConfig.ExcludeIdentifier, inclusionConfig.RequiredFeatureSet, inclusionConfig.Profile, clusterVersionCapabilitiesStatus, inclusionConfig.Overrides); err != nil {
468+
klog.V(4).Infof("Excluding %s: %s", ms[i].String(), err)
469+
ms = append(ms[:i], ms[i+1:]...)
470+
}
471+
}
454472
}
455473
}
456474

@@ -501,6 +519,7 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
501519
return true, nil
502520
}
503521
opts.TarEntryCallback = manifestReceiver.TarEntryCallback
522+
opts.TarEntryCallbackDoneCallback = manifestReceiver.TarEntryCallbackDoneCallback
504523
}
505524

506525
fileFound := false
@@ -550,6 +569,70 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
550569

551570
}
552571

572+
func currentPayloadManifests(ctx context.Context, opts *extract.ExtractOptions, config *rest.Config, inclusionConfig manifestInclusionConfiguration) ([]manifest.Manifest, error) {
573+
var currentPayloadManifests []manifest.Manifest
574+
optsToGetCurrentPayloadManifests, err := getOptsToGetCurrentPayloadManifests(ctx, opts, config)
575+
if err != nil {
576+
return nil, fmt.Errorf("error getting opts to get current payload manifests: %w", err)
577+
}
578+
optsToGetCurrentPayloadManifests.TarEntryCallback = func(h *tar.Header, _ extract.LayerInfo, r io.Reader) (cont bool, err error) {
579+
if ext := path.Ext(h.Name); len(ext) == 0 || !(ext == ".yaml" || ext == ".yml" || ext == ".json") {
580+
return true, nil
581+
}
582+
klog.V(4).Infof("Found manifest %s in the current release payload", h.Name)
583+
ms, err := manifest.ParseManifests(r)
584+
if err != nil {
585+
return false, err
586+
}
587+
for i := len(ms) - 1; i >= 0; i-- {
588+
if err := ms[i].Include(inclusionConfig.ExcludeIdentifier, inclusionConfig.RequiredFeatureSet, inclusionConfig.Profile, inclusionConfig.Capabilities, inclusionConfig.Overrides); err != nil {
589+
klog.V(4).Infof("Excluding %s in the current release payload: %s", ms[i].String(), err)
590+
ms = append(ms[:i], ms[i+1:]...)
591+
}
592+
}
593+
currentPayloadManifests = append(currentPayloadManifests, ms...)
594+
return true, nil
595+
}
596+
if err := optsToGetCurrentPayloadManifests.Run(); err != nil {
597+
return nil, fmt.Errorf("error getting current payload manifests: %w", err)
598+
}
599+
return currentPayloadManifests, nil
600+
}
601+
602+
func getOptsToGetCurrentPayloadManifests(ctx context.Context, source *extract.ExtractOptions, config *rest.Config) (*extract.ExtractOptions, error) {
603+
client, err := configv1client.NewForConfig(config)
604+
if err != nil {
605+
return nil, err
606+
}
607+
608+
clusterVersion, err := client.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
609+
if err != nil {
610+
return nil, err
611+
}
612+
613+
src := clusterVersion.Status.Desired.Image
614+
ref, err := imagesource.ParseReference(src)
615+
if err != nil {
616+
return nil, err
617+
}
618+
klog.V(4).Infof("The outgoing release payload from %s is running on the cluster: %s", src, config.Host)
619+
opts := extract.NewExtractOptions(genericiooptions.IOStreams{Out: source.Out, ErrOut: source.ErrOut})
620+
opts.ParallelOptions = source.ParallelOptions
621+
opts.SecurityOptions = source.SecurityOptions
622+
opts.FilterOptions = source.FilterOptions
623+
opts.FileDir = source.FileDir
624+
opts.OnlyFiles = true
625+
opts.ICSPFile = source.ICSPFile
626+
opts.IDMSFile = source.IDMSFile
627+
opts.Mappings = []extract.Mapping{
628+
{
629+
ImageRef: ref,
630+
From: "release-manifests/",
631+
},
632+
}
633+
return opts, nil
634+
}
635+
553636
func (o *ExtractOptions) extractGit(dir string) error {
554637
switch o.Output {
555638
case "commit", "":

‎pkg/cli/admin/release/extract_tools.go‎

Lines changed: 137 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"os"
1616
"path"
1717
"path/filepath"
18-
"regexp"
1918
"runtime"
2019
"sort"
2120
"strings"
@@ -28,6 +27,7 @@ import (
2827
terminal "golang.org/x/term"
2928

3029
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
kerrors "k8s.io/apimachinery/pkg/util/errors"
3131
"k8s.io/apimachinery/pkg/util/sets"
3232
"k8s.io/cli-runtime/pkg/genericiooptions"
3333
appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
@@ -1248,20 +1248,17 @@ func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config, vers
12481248
return config, err
12491249
} else {
12501250
config.Overrides = clusterVersion.Spec.Overrides
1251-
config.Capabilities = &clusterVersion.Status.Capabilities
1252-
1253-
// 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..
1254-
currentVersion := clusterVersion.Status.Desired.Version
1255-
matches := regexp.MustCompile(`^(\d+[.]\d+)[.].*`).FindStringSubmatch(currentVersion)
1256-
if len(matches) < 2 {
1257-
return config, fmt.Errorf("failed to parse major.minor version from ClusterVersion status.desired.version %q", currentVersion)
1258-
} else if matches[1] == "4.13" {
1259-
build := configv1.ClusterVersionCapability("Build")
1260-
deploymentConfig := configv1.ClusterVersionCapability("DeploymentConfig")
1261-
imageRegistry := configv1.ClusterVersionCapability("ImageRegistry")
1262-
config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry)
1263-
config.Capabilities.KnownCapabilities = append(config.Capabilities.KnownCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry)
1251+
1252+
// known and baseline may grow from the current cluster version to the oc version
1253+
capSet := configv1.ClusterVersionCapabilitySetCurrent
1254+
if capabilitiesSpec := clusterVersion.Spec.Capabilities; capabilitiesSpec != nil &&
1255+
len(capabilitiesSpec.BaselineCapabilitySet) > 0 {
1256+
capSet = capabilitiesSpec.BaselineCapabilitySet
12641257
}
1258+
deepCopy := clusterVersion.Status.Capabilities.DeepCopy()
1259+
deepCopy.EnabledCapabilities = append(deepCopy.EnabledCapabilities, configv1.ClusterVersionCapabilitySets[capSet]...)
1260+
deepCopy.KnownCapabilities = configv1.KnownClusterVersionCapabilities
1261+
config.Capabilities = deepCopy
12651262

12661263
if err := logCapabilitySetMayDiffer(clusterVersion.Spec.Capabilities.BaselineCapabilitySet, versionInImageConfig); err != nil {
12671264
return config, err
@@ -1308,16 +1305,36 @@ func newIncluder(config manifestInclusionConfiguration) includer {
13081305
// * with the manifests from every file whose name is not in skipNames, OR
13091306
// * with the reader that contains the content of each file whose name is in skipNames.
13101307
// All the errors encountered when parsing the manifests are collected in ManifestErrs.
1308+
// If needEnabledCapabilities is not set, manifestsCallback with the argument
1309+
// enabledCapabilities always nil is called right after each TarEntryCallback call from the upstream.
1310+
// Otherwise, manifestsCallback is called only after TarEntryCallbackDoneCallback is invoked by the upstream. By then,
1311+
// all the manifests have been handled. Those manifests, as the payloads of a release to update a cluster, together with
1312+
// the given currentPayloadManifests and manifestInclusionConfiguration, are used to calculate the enabled capabilities
1313+
// after the update. Those enabled capabilities is passed to manifestsCallback.
13111314
type ManifestReceiver struct {
1312-
manifestsCallback func(filename string, manifests []manifest.Manifest, reader io.Reader) (cont bool, err error)
1313-
skipNames sets.Set[string]
1315+
manifestsCallback func(filename string, manifests []manifest.Manifest, reader io.Reader, enabledCapabilities []configv1.ClusterVersionCapability) (cont bool, err error)
1316+
skipNames sets.Set[string]
1317+
needEnabledCapabilities bool
1318+
currentPayloadManifests []manifest.Manifest
1319+
inclusionConfiguration manifestInclusionConfiguration
1320+
1321+
cache []cacheData
1322+
manifests []manifest.Manifest
1323+
enabledResolved bool
1324+
enabled []configv1.ClusterVersionCapability
13141325

13151326
ManifestErrs []error
13161327
}
13171328

1329+
type cacheData struct {
1330+
name string
1331+
ms []manifest.Manifest
1332+
reader io.Reader
1333+
}
1334+
13181335
func (mr *ManifestReceiver) TarEntryCallback(h *tar.Header, _ extract.LayerInfo, r io.Reader) (cont bool, err error) {
13191336
if mr.skipNames.Has(h.Name) {
1320-
return mr.manifestsCallback(h.Name, nil, r)
1337+
return mr.manifestsCallback(h.Name, nil, r, nil)
13211338
}
13221339

13231340
if ext := path.Ext(h.Name); len(ext) == 0 || !(ext == ".yaml" || ext == ".yml" || ext == ".json") {
@@ -1329,5 +1346,107 @@ func (mr *ManifestReceiver) TarEntryCallback(h *tar.Header, _ extract.LayerInfo,
13291346
mr.ManifestErrs = append(mr.ManifestErrs, errors.Wrapf(err, "error parsing %s", h.Name))
13301347
return true, nil
13311348
}
1332-
return mr.manifestsCallback(h.Name, ms, nil)
1349+
mr.manifests = append(mr.manifests, ms...)
1350+
if mr.needEnabledCapabilities {
1351+
mr.cache = append(mr.cache, cacheData{name: h.Name, ms: ms})
1352+
return true, nil
1353+
}
1354+
1355+
return mr.manifestsCallback(h.Name, ms, nil, nil)
1356+
}
1357+
1358+
func (mr *ManifestReceiver) TarEntryCallbackDoneCallback() error {
1359+
defer func() {
1360+
mr.cache = []cacheData{}
1361+
}()
1362+
1363+
if mr.needEnabledCapabilities && !mr.enabledResolved {
1364+
enabled := GetImplicitlyEnabledCapabilities(
1365+
mr.manifests,
1366+
mr.currentPayloadManifests,
1367+
mr.inclusionConfiguration,
1368+
sets.New[configv1.ClusterVersionCapability](),
1369+
)
1370+
1371+
delta := enabled.Clone()
1372+
if mr.inclusionConfiguration.Capabilities != nil {
1373+
delta = delta.Difference(sets.New[configv1.ClusterVersionCapability](mr.inclusionConfiguration.Capabilities.EnabledCapabilities...))
1374+
enabled.Insert(mr.inclusionConfiguration.Capabilities.EnabledCapabilities...)
1375+
}
1376+
if delta.Len() > 0 {
1377+
klog.Infof("Those capabilities become implicitly enabled for the incoming release %s", sets.List(delta))
1378+
}
1379+
1380+
mr.enabled = enabled.UnsortedList()
1381+
mr.enabledResolved = true
1382+
}
1383+
1384+
for _, c := range mr.cache {
1385+
cont, err := mr.manifestsCallback(c.name, c.ms, c.reader, mr.enabled)
1386+
if err != nil {
1387+
mr.ManifestErrs = append(mr.ManifestErrs, errors.Wrapf(err, "error parsing %s", c.name))
1388+
}
1389+
if !cont {
1390+
break
1391+
}
1392+
}
1393+
return kerrors.NewAggregate(mr.ManifestErrs)
1394+
}
1395+
1396+
// GetImplicitlyEnabledCapabilities returns a set of capabilities that are implicitly enabled after a cluster update.
1397+
// The arguments are two sets of manifests, manifest inclusion configuration, and
1398+
// a set of capabilities that are implicitly enabled on the cluster, i.e., the capabilities
1399+
// that are NOT specified in the cluster version but has to considered enabled on the cluster.
1400+
// The manifest inclusion configuration is used to determine if a manifest should be included.
1401+
// In other words, whether, or not the cluster version operator reconcile that manifest on the cluster.
1402+
// The two sets of manifests are respectively from the release that is currently running on the cluster and
1403+
// from the release that the cluster is updated to.
1404+
// TODO lift this function to library-go
1405+
func GetImplicitlyEnabledCapabilities(
1406+
updatePayloadManifests []manifest.Manifest,
1407+
currentPayloadManifests []manifest.Manifest,
1408+
manifestInclusionConfiguration manifestInclusionConfiguration,
1409+
currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability],
1410+
) sets.Set[configv1.ClusterVersionCapability] {
1411+
ret := currentImplicitlyEnabled.Clone()
1412+
for _, updateManifest := range updatePayloadManifests {
1413+
updateManErr := updateManifest.IncludeAllowUnknownCapabilities(
1414+
manifestInclusionConfiguration.ExcludeIdentifier,
1415+
manifestInclusionConfiguration.RequiredFeatureSet,
1416+
manifestInclusionConfiguration.Profile,
1417+
manifestInclusionConfiguration.Capabilities,
1418+
manifestInclusionConfiguration.Overrides,
1419+
true,
1420+
)
1421+
// update manifest is enabled, no need to check
1422+
if updateManErr == nil {
1423+
continue
1424+
}
1425+
for _, currentManifest := range currentPayloadManifests {
1426+
if !updateManifest.SameResourceID(currentManifest) {
1427+
continue
1428+
}
1429+
// current manifest is disabled, no need to check
1430+
if err := currentManifest.IncludeAllowUnknownCapabilities(
1431+
manifestInclusionConfiguration.ExcludeIdentifier,
1432+
manifestInclusionConfiguration.RequiredFeatureSet,
1433+
manifestInclusionConfiguration.Profile,
1434+
manifestInclusionConfiguration.Capabilities,
1435+
manifestInclusionConfiguration.Overrides,
1436+
true,
1437+
); err != nil {
1438+
continue
1439+
}
1440+
newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...).
1441+
Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)).
1442+
Difference(currentImplicitlyEnabled).
1443+
Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...))
1444+
ret = ret.Union(newImplicitlyEnabled)
1445+
if newImplicitlyEnabled.Len() > 0 {
1446+
klog.V(2).Infof("%s has changed and is now part of one or more disabled capabilities. The following capabilities will be implicitly enabled: %s",
1447+
updateManifest.String(), sets.List(newImplicitlyEnabled))
1448+
}
1449+
}
1450+
}
1451+
return ret
13331452
}

‎pkg/cli/image/extract/extract.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ type ExtractOptions struct {
150150
// by name and only the entry in the highest layer will be passed to the callback. Returning false
151151
// will halt processing of the image.
152152
TarEntryCallback TarEntryFunc
153+
// TarEntryCallbackDoneCallback, if set, is called when all layers image have been handled, i.e., no more
154+
// TarEntryCallback is going to be passed. It has no effect if TarEntryCallback is not set.
155+
TarEntryCallbackDoneCallback func() error
153156
// AllLayers ensures the TarEntryCallback is invoked for all files, and will cause the callback
154157
// order to start at the lowest layer and work outwards.
155158
AllLayers bool
@@ -549,6 +552,12 @@ func (o *ExtractOptions) Run() error {
549552
}
550553
}
551554

555+
if o.TarEntryCallback != nil && o.TarEntryCallbackDoneCallback != nil {
556+
if err := o.TarEntryCallbackDoneCallback(); err != nil {
557+
return err
558+
}
559+
}
560+
552561
if o.ImageMetadataCallback != nil {
553562
o.ImageMetadataCallback(&mapping, location.Manifest, contentDigest, imageConfig, location.ManifestListDigest())
554563
}

0 commit comments

Comments
 (0)