Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hack/cluster-version-util/task_graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func newTaskGraphCmd() *cobra.Command {

func runTaskGraphCmd(cmd *cobra.Command, args []string) error {
manifestDir := args[0]
release, err := payload.LoadUpdate(manifestDir, "", "", payload.DefaultClusterProfile)
release, err := payload.LoadUpdate(manifestDir, "", "", nil, payload.DefaultClusterProfile)
if err != nil {
return err
}
Expand Down
11 changes: 10 additions & 1 deletion pkg/cvo/cvo.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,15 @@ func New(
// controller that loads and applies content to the cluster. It returns an error if the payload appears to
// be in error rather than continuing.
func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestConfig *rest.Config) error {
update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, optr.clusterProfile)
includeTechPreview := func() (bool, error) {
gate, err := optr.client.ConfigV1().FeatureGates().Get(context.TODO(), "cluster", metav1.GetOptions{})
Comment thread
wking marked this conversation as resolved.
Outdated
if err != nil && !apierrors.IsNotFound(err) {
return false, err
}
return gate.Spec.FeatureSet == configv1.TechPreviewNoUpgrade, nil
}

update, err := payload.LoadUpdate(optr.defaultPayloadDir(), optr.release.Image, optr.exclude, includeTechPreview, optr.clusterProfile)
if err != nil {
return fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err)
}
Expand Down Expand Up @@ -264,6 +272,7 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
Steps: 3,
},
optr.exclude,
includeTechPreview,
optr.eventRecorder,
optr.clusterProfile,
)
Expand Down
8 changes: 8 additions & 0 deletions pkg/cvo/cvo_scenarios_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]runtime.Object, *fak
}
return false, nil, fmt.Errorf("unexpected client action: %#v", action)
})
client.AddReactor("get", "featuregates", func(action clientgotesting.Action) (handled bool, ret runtime.Object, err error) {
switch a := action.(type) {
case clientgotesting.GetAction:
return true, nil, errors.NewNotFound(schema.GroupResource{Resource: "clusterversions"}, a.GetName())
}
return false, nil, nil
})

o := &Operator{
namespace: "test",
Expand All @@ -97,6 +104,7 @@ func setupCVOTest(payloadDir string) (*Operator, map[string]runtime.Object, *fak
Steps: 1,
},
"exclude-test",
nil,
record.NewFakeRecorder(100),
o.clusterProfile,
)
Expand Down
13 changes: 8 additions & 5 deletions pkg/cvo/sync_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,14 @@ type SyncWorker struct {
// of the form exclude.release.openshift.io/<identifier>=true
exclude string

includeTechPreview func() (bool, error)

clusterProfile string
}

// NewSyncWorker initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
// to a server, and obey limits about how often to reconcile or retry on errors.
func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, includeTechPreview func() (bool, error), eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
return &SyncWorker{
retriever: retriever,
builder: builder,
Expand All @@ -175,7 +177,8 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
// if the reader is not fast enough.
report: make(chan SyncWorkerStatus, 500),

exclude: exclude,
exclude: exclude,
includeTechPreview: includeTechPreview,

clusterProfile: clusterProfile,
}
Expand All @@ -184,8 +187,8 @@ func NewSyncWorker(retriever PayloadRetriever, builder payload.ResourceBuilder,
// NewSyncWorkerWithPreconditions initializes a ConfigSyncWorker that will retrieve payloads to disk, apply them via builder
// to a server, and obey limits about how often to reconcile or retry on errors.
// It allows providing preconditions for loading payload.
func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, eventRecorder, clusterProfile)
func NewSyncWorkerWithPreconditions(retriever PayloadRetriever, builder payload.ResourceBuilder, preconditions precondition.List, reconcileInterval time.Duration, backoff wait.Backoff, exclude string, includeTechPreview func() (bool, error), eventRecorder record.EventRecorder, clusterProfile string) *SyncWorker {
worker := NewSyncWorker(retriever, builder, reconcileInterval, backoff, exclude, includeTechPreview, eventRecorder, clusterProfile)
worker.preconditions = preconditions
return worker
}
Expand Down Expand Up @@ -576,7 +579,7 @@ func (w *SyncWorker) syncOnce(ctx context.Context, work *SyncWork, maxWorkers in
}

w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeNormal, "VerifyPayload", "verifying payload version=%q image=%q", desired.Version, desired.Image)
payloadUpdate, err := payload.LoadUpdate(info.Directory, desired.Image, w.exclude, w.clusterProfile)
payloadUpdate, err := payload.LoadUpdate(info.Directory, desired.Image, w.exclude, w.includeTechPreview, w.clusterProfile)
if err != nil {
w.eventRecorder.Eventf(cvoObjectRef, corev1.EventTypeWarning, "VerifyPayloadFailed", "verifying payload failed version=%q image=%q failure=%v", desired.Version, desired.Image, err)
reporter.Report(SyncWorkerStatus{
Expand Down
19 changes: 16 additions & 3 deletions pkg/payload/payload.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,20 @@ type metadata struct {
Metadata map[string]interface{}
}

func LoadUpdate(dir, releaseImage, excludeIdentifier, profile string) (*Update, error) {
func LoadUpdate(dir, releaseImage, excludeIdentifier string, includeTechPreview func() (bool, error), profile string) (*Update, error) {
payload, tasks, err := loadUpdatePayloadMetadata(dir, releaseImage, profile)
if err != nil {
return nil, err
}

techPreviews := false
if includeTechPreview != nil {
techPreviews, err = includeTechPreview()
if err != nil {
return nil, err
}
}

var manifests []manifest.Manifest
var errs []error
for _, task := range tasks {
Expand Down Expand Up @@ -182,7 +190,7 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier, profile string) (*Update,
// Filter out manifests that should be excluded based on annotation
filteredMs := []manifest.Manifest{}
for _, manifest := range ms {
if shouldExclude(excludeIdentifier, profile, &manifest) {
if shouldExclude(excludeIdentifier, profile, &manifest, techPreviews) {
continue
}
filteredMs = append(filteredMs, manifest)
Expand Down Expand Up @@ -213,7 +221,7 @@ func LoadUpdate(dir, releaseImage, excludeIdentifier, profile string) (*Update,
return payload, nil
}

func shouldExclude(excludeIdentifier, profile string, manifest *manifest.Manifest) bool {
func shouldExclude(excludeIdentifier, profile string, manifest *manifest.Manifest, includeTechPreview bool) bool {
Comment thread
wking marked this conversation as resolved.
Outdated
annotations := manifest.Obj.GetAnnotations()
if annotations == nil {
return true
Expand All @@ -224,6 +232,11 @@ func shouldExclude(excludeIdentifier, profile string, manifest *manifest.Manifes
return true
}

techPreviewAnnotation := "featuregate.release.openshift.io/tech-preview"
if annotations[techPreviewAnnotation] == "true" && !includeTechPreview {
return true
}

profileAnnotation := fmt.Sprintf("include.release.openshift.io/%s", profile)
if val, ok := annotations[profileAnnotation]; ok && val == "true" {
return false
Expand Down
4 changes: 2 additions & 2 deletions pkg/payload/payload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func Test_loadUpdatePayload(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := LoadUpdate(tt.args.dir, tt.args.releaseImage, "exclude-test", DefaultClusterProfile)
got, err := LoadUpdate(tt.args.dir, tt.args.releaseImage, "exclude-test", nil, DefaultClusterProfile)
if (err != nil) != tt.wantErr {
t.Errorf("loadUpdatePayload() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down Expand Up @@ -179,7 +179,7 @@ func Test_Exclude(t *testing.T) {
"metadata": metadata,
},
},
})
}, false)
if ret != tt.isExcluded {
t.Errorf("(exclude: %v, profile: %v, annotations: %v) %v != %v", tt.exclude, tt.profile, tt.annotations, tt.isExcluded, ret)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/payload/task_graph_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func Test_TaskGraph_real(t *testing.T) {
if len(path) == 0 {
t.Skip("TEST_GRAPH_PATH unset")
}
p, err := LoadUpdate(path, "arbitrary/image:1", "", DefaultClusterProfile)
p, err := LoadUpdate(path, "arbitrary/image:1", "", nil, DefaultClusterProfile)
if err != nil {
t.Fatal(err)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/start/start_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func TestIntegrationCVO_initializeAndUpgrade(t *testing.T) {
options.PayloadOverride = filepath.Join(dir, "ignored")
controllers := options.NewControllerContext(cb)

worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", nil, record.NewFakeRecorder(100), payload.DefaultClusterProfile)
controllers.CVO.SetSyncWorkerForTesting(worker)

lock, err := createResourceLock(cb, options.Namespace, options.Name)
Expand Down Expand Up @@ -448,7 +448,7 @@ func TestIntegrationCVO_initializeAndHandleError(t *testing.T) {
options.ResyncInterval = 3 * time.Second
controllers := options.NewControllerContext(cb)

worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Duration: time.Second, Factor: 1.2}, "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Duration: time.Second, Factor: 1.2}, "", nil, record.NewFakeRecorder(100), payload.DefaultClusterProfile)
controllers.CVO.SetSyncWorkerForTesting(worker)

lock, err := createResourceLock(cb, options.Namespace, options.Name)
Expand Down Expand Up @@ -563,7 +563,7 @@ func TestIntegrationCVO_gracefulStepDown(t *testing.T) {
options.NodeName = "test-node"
controllers := options.NewControllerContext(cb)

worker := cvo.NewSyncWorker(&mapPayloadRetriever{}, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
worker := cvo.NewSyncWorker(&mapPayloadRetriever{}, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", nil, record.NewFakeRecorder(100), payload.DefaultClusterProfile)
controllers.CVO.SetSyncWorkerForTesting(worker)

lock, err := createResourceLock(cb, options.Namespace, options.Name)
Expand Down Expand Up @@ -754,7 +754,7 @@ metadata:
t.Fatal(err)
}

worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", record.NewFakeRecorder(100), payload.DefaultClusterProfile)
worker := cvo.NewSyncWorker(retriever, cvo.NewResourceBuilder(cfg, cfg, nil, nil), 5*time.Second, wait.Backoff{Steps: 3}, "", nil, record.NewFakeRecorder(100), payload.DefaultClusterProfile)
controllers.CVO.SetSyncWorkerForTesting(worker)

lock, err := createResourceLock(cb, options.Namespace, options.Name)
Expand Down