diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 644c882c8f..4e2c921cc3 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -961,16 +961,27 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { // not-satisfiable error if _, ok := err.(solver.NotSatisfiable); ok { logger.WithError(err).Debug("resolution failed") - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ConstraintsNotSatisfiable", err.Error(), true) - _, updateErr := o.updateSubscriptionStatuses(subs) + _, updateErr := o.updateSubscriptionStatuses( + o.setSubsCond(subs, v1alpha1.SubscriptionCondition{ + Type: v1alpha1.SubscriptionResolutionFailed, + Reason: "ConstraintsNotSatisfiable", + Message: err.Error(), + Status: corev1.ConditionTrue, + })) if updateErr != nil { logger.WithError(updateErr).Debug("failed to update subs conditions") return updateErr } return nil } - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ErrorPreventedResolution", err.Error(), true) - _, updateErr := o.updateSubscriptionStatuses(subs) + + _, updateErr := o.updateSubscriptionStatuses( + o.setSubsCond(subs, v1alpha1.SubscriptionCondition{ + Type: v1alpha1.SubscriptionResolutionFailed, + Reason: "ErrorPreventedResolution", + Message: err.Error(), + Status: corev1.ConditionTrue, + })) if updateErr != nil { logger.WithError(updateErr).Debug("failed to update subs conditions") return updateErr @@ -978,14 +989,6 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { return err } - defer func() { - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "", "", false) - _, updateErr := o.updateSubscriptionStatuses(subs) - if updateErr != nil { - logger.WithError(updateErr).Warn("failed to update subscription conditions") - } - }() - // create installplan if anything updated if len(updatedSubs) > 0 { logger.Debug("resolution caused subscription changes, creating installplan") @@ -1016,17 +1019,36 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { return err } updatedSubs = o.setIPReference(updatedSubs, maxGeneration+1, installPlanReference) - for _, updatedSub := range updatedSubs { - for i, sub := range subs { - if sub.Name == updatedSub.Name && sub.Namespace == updatedSub.Namespace { - subs[i] = updatedSub - } - } - } } else { logger.Debugf("no subscriptions were updated") } + // Remove resolutionfailed condition from subscriptions + subs = o.removeSubsCond(subs, v1alpha1.SubscriptionResolutionFailed) + newSub := true + for _, updatedSub := range updatedSubs { + updatedSub.Status.RemoveConditions(v1alpha1.SubscriptionResolutionFailed) + for i, sub := range subs { + if sub.Name == updatedSub.Name && sub.Namespace == updatedSub.Namespace { + subs[i] = updatedSub + newSub = false + break + } + } + if newSub { + subs = append(subs, updatedSub) + continue + } + newSub = true + } + + // Update subscriptions with all changes so far + _, updateErr := o.updateSubscriptionStatuses(subs) + if updateErr != nil { + logger.WithError(updateErr).Warn("failed to update subscription conditions") + return updateErr + } + return nil } @@ -1043,11 +1065,6 @@ func (o *Operator) syncSubscriptions(obj interface{}) error { } func (o *Operator) nothingToUpdate(logger *logrus.Entry, sub *v1alpha1.Subscription) bool { - // Only sync if catalog has been updated since last sync time - if o.sourcesLastUpdate.Before(sub.Status.LastUpdated.Time) && sub.Status.State != v1alpha1.SubscriptionStateNone && sub.Status.State != v1alpha1.SubscriptionStateUpgradeAvailable { - logger.Debugf("skipping update: no new updates to catalog since last sync at %s", sub.Status.LastUpdated.String()) - return true - } if sub.Status.InstallPlanRef != nil && sub.Status.State == v1alpha1.SubscriptionStateUpgradePending { logger.Debugf("skipping update: installplan already created") return true @@ -1088,12 +1105,7 @@ func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub out.Status.CurrentCSV = out.Spec.StartingCSV out.Status.LastUpdated = o.now() - updated, err := o.client.OperatorsV1alpha1().Subscriptions(sub.GetNamespace()).UpdateStatus(context.TODO(), out, metav1.UpdateOptions{}) - if err != nil { - return nil, false, err - } - - return updated, true, nil + return out, true, nil } func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha1.Subscription, querier resolver.SourceQuerier) (*v1alpha1.Subscription, bool, error) { @@ -1101,7 +1113,7 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha return sub, false, nil } - csv, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(sub.GetNamespace()).Get(context.TODO(), sub.Status.CurrentCSV, metav1.GetOptions{}) + _, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(sub.GetNamespace()).Get(context.TODO(), sub.Status.CurrentCSV, metav1.GetOptions{}) out := sub.DeepCopy() if err != nil { logger.WithError(err).WithField("currentCSV", sub.Status.CurrentCSV).Debug("error fetching csv listed in subscription status") @@ -1111,14 +1123,7 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha if err := querier.Queryable(); err != nil { return nil, false, err } - b, _, _ := querier.FindReplacement(&csv.Spec.Version.Version, sub.Status.CurrentCSV, sub.Spec.Package, sub.Spec.Channel, registry.CatalogKey{Name: sub.Spec.CatalogSource, Namespace: sub.Spec.CatalogSourceNamespace}) - if b != nil { - o.logger.Tracef("replacement %s bundle found for current bundle %s", b.CsvName, sub.Status.CurrentCSV) - out.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable - } else { - out.Status.State = v1alpha1.SubscriptionStateAtLatest - } - + out.Status.State = v1alpha1.SubscriptionStateAtLatest out.Status.InstalledCSV = sub.Status.CurrentCSV } @@ -1236,23 +1241,45 @@ func (o *Operator) createInstallPlan(namespace string, gen int, subs []*v1alpha1 return reference.GetReference(res) } -func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType, reason, message string, setTrue bool) []*v1alpha1.Subscription { +// setSubsCond will set the condition to the subscription if it doesn't already +// exist or if it is different +// Only return the list of updated subscriptions +func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, cond v1alpha1.SubscriptionCondition) []*v1alpha1.Subscription { var ( lastUpdated = o.now() + subList []*v1alpha1.Subscription ) + for _, sub := range subs { + subCond := sub.Status.GetCondition(cond.Type) + if subCond.Equals(cond) { + continue + } sub.Status.LastUpdated = lastUpdated + sub.Status.SetCondition(cond) + subList = append(subList, sub) + } + return subList +} + +// removeSubsCond will remove the condition to the subscription if it exists +// Only return the list of updated subscriptions +func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) []*v1alpha1.Subscription { + var ( + lastUpdated = o.now() + ) + var subList []*v1alpha1.Subscription + for _, sub := range subs { cond := sub.Status.GetCondition(condType) - cond.Reason = reason - cond.Message = message - if setTrue { - cond.Status = corev1.ConditionTrue - } else { - cond.Status = corev1.ConditionFalse + // if status is ConditionUnknown, the condition doesn't exist. Just skip + if cond.Status == corev1.ConditionUnknown { + continue } - sub.Status.SetCondition(cond) + sub.Status.LastUpdated = lastUpdated + sub.Status.RemoveConditions(condType) + subList = append(subList, sub) } - return subs + return subList } func (o *Operator) updateSubscriptionStatuses(subs []*v1alpha1.Subscription) ([]*v1alpha1.Subscription, error) { diff --git a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscriptions_test.go b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscriptions_test.go index 9d3d2212f3..0a663f0a9e 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscriptions_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/operators/catalog/subscriptions_test.go @@ -155,14 +155,6 @@ func TestSyncSubscriptions(t *testing.T) { }, LastUpdated: now, InstallPlanGeneration: 1, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, @@ -306,14 +298,6 @@ func TestSyncSubscriptions(t *testing.T) { }, LastUpdated: now, InstallPlanGeneration: 1, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, @@ -462,14 +446,6 @@ func TestSyncSubscriptions(t *testing.T) { }, InstallPlanGeneration: 1, LastUpdated: now, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, @@ -623,14 +599,6 @@ func TestSyncSubscriptions(t *testing.T) { }, LastUpdated: now, InstallPlanGeneration: 1, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, @@ -807,14 +775,6 @@ func TestSyncSubscriptions(t *testing.T) { }, LastUpdated: now, InstallPlanGeneration: 1, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, @@ -998,14 +958,6 @@ func TestSyncSubscriptions(t *testing.T) { }, LastUpdated: now, InstallPlanGeneration: 2, - Conditions: []v1alpha1.SubscriptionCondition{ - { - Type: "ResolutionFailed", - Status: corev1.ConditionFalse, - Reason: "", - Message: "", - }, - }, }, }, }, diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go b/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go index 7acbc8a8a3..121c2563f4 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go @@ -31,6 +31,7 @@ type SourceQuerier interface { FindProvider(api opregistry.APIKey, initialSource registry.CatalogKey, excludedPackages map[string]struct{}) (*api.Bundle, *registry.CatalogKey, error) FindBundle(pkgName, channelName, bundleName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) FindLatestBundle(pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) + // Deprecated: This FindReplacement function will be deprecated soon FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) Queryable() error } @@ -123,6 +124,7 @@ func (q *NamespaceSourceQuerier) FindLatestBundle(pkgName, channelName string, i return nil, nil, fmt.Errorf("%s/%s not found in any available CatalogSource", pkgName, channelName) } +// Deprecated: This FindReplacement function will be deprecated soon func (q *NamespaceSourceQuerier) FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) { errs := []error{} diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier_test.go b/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier_test.go index 20d1c0e700..53b31e29a1 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/resolver/querier_test.go @@ -2,19 +2,14 @@ package resolver import ( "context" - "encoding/json" "fmt" "testing" - "github.com/blang/semver/v4" "github.com/operator-framework/operator-registry/pkg/api" "github.com/operator-framework/operator-registry/pkg/client" opregistry "github.com/operator-framework/operator-registry/pkg/registry" "github.com/stretchr/testify/require" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "github.com/operator-framework/api/pkg/lib/version" - "github.com/operator-framework/api/pkg/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/fakes" ) @@ -344,178 +339,3 @@ func TestNamespaceSourceQuerier_FindPackage(t *testing.T) { }) } } - -func TestNamespaceSourceQuerier_FindReplacement(t *testing.T) { - // TODO: clean up this test setup - initialSource := fakes.FakeClientInterface{} - otherSource := fakes.FakeClientInterface{} - replacementSource := fakes.FakeClientInterface{} - replacementAndLatestSource := fakes.FakeClientInterface{} - replacementAndNoAnnotationLatestSource := fakes.FakeClientInterface{} - - latestVersion := semver.MustParse("1.0.0-1556661308") - csv := v1alpha1.ClusterServiceVersion{ - TypeMeta: metav1.TypeMeta{ - Kind: v1alpha1.ClusterServiceVersionKind, - APIVersion: v1alpha1.GroupVersion, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "latest", - Namespace: "placeholder", - Annotations: map[string]string{ - "olm.skipRange": ">= 1.0.0-0 < 1.0.0-1556661308", - }, - }, - Spec: v1alpha1.ClusterServiceVersionSpec{ - CustomResourceDefinitions: v1alpha1.CustomResourceDefinitions{ - Owned: []v1alpha1.CRDDescription{}, - Required: []v1alpha1.CRDDescription{}, - }, - APIServiceDefinitions: v1alpha1.APIServiceDefinitions{ - Owned: []v1alpha1.APIServiceDescription{}, - Required: []v1alpha1.APIServiceDescription{}, - }, - Version: version.OperatorVersion{latestVersion}, - }, - } - csvJson, err := json.Marshal(csv) - require.NoError(t, err) - - nextBundle := &api.Bundle{CsvName: "test.v1", PackageName: "testPkg", ChannelName: "testChannel"} - latestBundle := &api.Bundle{CsvName: "latest", PackageName: "testPkg", ChannelName: "testChannel", CsvJson: string(csvJson), Object: []string{string(csvJson)}, SkipRange: ">= 1.0.0-0 < 1.0.0-1556661308", Version: latestVersion.String()} - - csv.SetAnnotations(map[string]string{}) - csvUnstNoAnnotationJson, err := json.Marshal(csv) - require.NoError(t, err) - latestBundleNoAnnotation := &api.Bundle{CsvName: "latest", PackageName: "testPkg", ChannelName: "testChannel", CsvJson: string(csvUnstNoAnnotationJson), Object: []string{string(csvUnstNoAnnotationJson)}} - - initialSource.GetReplacementBundleInPackageChannelStub = func(ctx context.Context, bundleName, pkgName, channelName string) (*api.Bundle, error) { - return nil, fmt.Errorf("not found") - } - replacementSource.GetReplacementBundleInPackageChannelStub = func(ctx context.Context, bundleName, pkgName, channelName string) (*api.Bundle, error) { - return nextBundle, nil - } - initialSource.GetBundleInPackageChannelStub = func(ctx context.Context, pkgName, channelName string) (*api.Bundle, error) { - if pkgName != latestBundle.PackageName { - return nil, fmt.Errorf("not found") - } - return latestBundle, nil - } - otherSource.GetBundleInPackageChannelStub = func(ctx context.Context, pkgName, channelName string) (*api.Bundle, error) { - if pkgName != latestBundle.PackageName { - return nil, fmt.Errorf("not found") - } - return latestBundle, nil - } - replacementAndLatestSource.GetReplacementBundleInPackageChannelStub = func(ctx context.Context, bundleName, pkgName, channelName string) (*api.Bundle, error) { - return nextBundle, nil - } - replacementAndLatestSource.GetBundleInPackageChannelStub = func(ctx context.Context, pkgName, channelName string) (*api.Bundle, error) { - return latestBundle, nil - } - replacementAndNoAnnotationLatestSource.GetReplacementBundleInPackageChannelStub = func(ctx context.Context, bundleName, pkgName, channelName string) (*api.Bundle, error) { - return nextBundle, nil - } - replacementAndNoAnnotationLatestSource.GetBundleInPackageChannelStub = func(ctx context.Context, pkgName, channelName string) (*api.Bundle, error) { - return latestBundleNoAnnotation, nil - } - - initialKey := registry.CatalogKey{"initial", "ns"} - otherKey := registry.CatalogKey{"other", "other"} - replacementKey := registry.CatalogKey{"replacement", "ns"} - replacementAndLatestKey := registry.CatalogKey{"replat", "ns"} - replacementAndNoAnnotationLatestKey := registry.CatalogKey{"replatbad", "ns"} - - sources := map[registry.CatalogKey]registry.ClientInterface{ - initialKey: &initialSource, - otherKey: &otherSource, - replacementKey: &replacementSource, - replacementAndLatestKey: &replacementAndLatestSource, - replacementAndNoAnnotationLatestKey: &replacementAndNoAnnotationLatestSource, - } - - startVersion := semver.MustParse("1.0.0-0") - notInRange := semver.MustParse("1.0.0-1556661347") - - type fields struct { - sources map[registry.CatalogKey]registry.ClientInterface - } - type args struct { - currentVersion *semver.Version - pkgName string - channelName string - bundleName string - initialSource registry.CatalogKey - } - type out struct { - bundle *api.Bundle - key *registry.CatalogKey - err error - } - tests := []struct { - name string - fields fields - args args - out out - }{ - { - name: "FindsLatestInPrimaryCatalog", - fields: fields{sources: sources}, - args: args{&startVersion, "testPkg", "testChannel", "test.v1", initialKey}, - out: out{bundle: latestBundle, key: &initialKey, err: nil}, - }, - { - name: "FindsLatestInSecondaryCatalog", - fields: fields{sources: sources}, - args: args{&startVersion, "testPkg", "testChannel", "test.v1", otherKey}, - out: out{bundle: latestBundle, key: &otherKey, err: nil}, - }, - { - name: "PrefersLatestToReplaced/SameCatalog", - fields: fields{sources: sources}, - args: args{&startVersion, "testPkg", "testChannel", "test.v1", replacementAndLatestKey}, - out: out{bundle: latestBundle, key: &replacementAndLatestKey, err: nil}, - }, - { - name: "PrefersLatestToReplaced/OtherCatalog", - fields: fields{sources: sources}, - args: args{&startVersion, "testPkg", "testChannel", "test.v1", initialKey}, - out: out{bundle: latestBundle, key: &initialKey, err: nil}, - }, - { - name: "IgnoresLatestWithoutAnnotation", - fields: fields{sources: sources}, - args: args{&startVersion, "testPkg", "testChannel", "test.v1", replacementAndNoAnnotationLatestKey}, - out: out{bundle: nextBundle, key: &replacementAndNoAnnotationLatestKey, err: nil}, - }, - { - name: "IgnoresLatestNotInRange", - fields: fields{sources: sources}, - args: args{¬InRange, "testPkg", "testChannel", "test.v1", replacementAndLatestKey}, - out: out{bundle: nextBundle, key: &replacementAndLatestKey, err: nil}, - }, - { - name: "IgnoresLatestAtLatest", - fields: fields{sources: sources}, - args: args{&latestVersion, "testPkg", "testChannel", "test.v1", otherKey}, - out: out{bundle: nil, key: nil, err: nil}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - q := &NamespaceSourceQuerier{ - sources: tt.fields.sources, - } - var got *api.Bundle - var key *registry.CatalogKey - var err error - got, key, err = q.FindReplacement(tt.args.currentVersion, tt.args.bundleName, tt.args.pkgName, tt.args.channelName, tt.args.initialSource) - if err != nil { - t.Log(err.Error()) - } - require.Equal(t, tt.out.err, err, "%v", err) - require.Equal(t, tt.out.bundle, got) - require.Equal(t, tt.out.key, key) - }) - } -} diff --git a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go index 85f1381bd5..7801e9e964 100644 --- a/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go +++ b/staging/operator-lifecycle-manager/test/e2e/subscription_e2e_test.go @@ -2067,7 +2067,7 @@ var _ = Describe("Subscription", func() { } updateInternalCatalog(GinkgoT(), kubeClient, crClient, catalogSourceName, testNamespace, []apiextensions.CustomResourceDefinition{crd, crd2}, []operatorsv1alpha1.ClusterServiceVersion{csvNewA, csvA, csvB}, manifests) csvAsub := strings.Join([]string{packageName1, stableChannel, catalogSourceName, testNamespace}, "-") - _, err = fetchSubscription(crClient, testNamespace, csvAsub, subscriptionStateUpgradeAvailableChecker) + _, err = fetchSubscription(crClient, testNamespace, csvAsub, subscriptionStateAtLatestChecker) require.NoError(GinkgoT(), err) // Ensure csvNewA is not installed _, err = crClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.Background(), csvNewA.Name, metav1.GetOptions{}) @@ -2174,14 +2174,14 @@ var _ = Describe("Subscription", func() { updateInternalCatalog(GinkgoT(), c, crc, catSrcName, testNamespace, []apiextensions.CustomResourceDefinition{crd}, []operatorsv1alpha1.ClusterServiceVersion{csvA, csvB}, packages) }) - It("the ResolutionFailed condition previously set in it's status that indicated the resolution error is cleared off", func() { + It("the ResolutionFailed condition previously set in its status that indicated the resolution error is cleared off", func() { Eventually(func() (corev1.ConditionStatus, error) { sub, err := crc.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.Background(), subName, metav1.GetOptions{}) if err != nil { - return corev1.ConditionUnknown, err + return corev1.ConditionFalse, err } return sub.Status.GetCondition(operatorsv1alpha1.SubscriptionResolutionFailed).Status, nil - }).Should(Equal(corev1.ConditionFalse)) + }).Should(Equal(corev1.ConditionUnknown)) }) }) }) diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go index 644c882c8f..4e2c921cc3 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/operators/catalog/operator.go @@ -961,16 +961,27 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { // not-satisfiable error if _, ok := err.(solver.NotSatisfiable); ok { logger.WithError(err).Debug("resolution failed") - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ConstraintsNotSatisfiable", err.Error(), true) - _, updateErr := o.updateSubscriptionStatuses(subs) + _, updateErr := o.updateSubscriptionStatuses( + o.setSubsCond(subs, v1alpha1.SubscriptionCondition{ + Type: v1alpha1.SubscriptionResolutionFailed, + Reason: "ConstraintsNotSatisfiable", + Message: err.Error(), + Status: corev1.ConditionTrue, + })) if updateErr != nil { logger.WithError(updateErr).Debug("failed to update subs conditions") return updateErr } return nil } - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "ErrorPreventedResolution", err.Error(), true) - _, updateErr := o.updateSubscriptionStatuses(subs) + + _, updateErr := o.updateSubscriptionStatuses( + o.setSubsCond(subs, v1alpha1.SubscriptionCondition{ + Type: v1alpha1.SubscriptionResolutionFailed, + Reason: "ErrorPreventedResolution", + Message: err.Error(), + Status: corev1.ConditionTrue, + })) if updateErr != nil { logger.WithError(updateErr).Debug("failed to update subs conditions") return updateErr @@ -978,14 +989,6 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { return err } - defer func() { - subs = o.setSubsCond(subs, v1alpha1.SubscriptionResolutionFailed, "", "", false) - _, updateErr := o.updateSubscriptionStatuses(subs) - if updateErr != nil { - logger.WithError(updateErr).Warn("failed to update subscription conditions") - } - }() - // create installplan if anything updated if len(updatedSubs) > 0 { logger.Debug("resolution caused subscription changes, creating installplan") @@ -1016,17 +1019,36 @@ func (o *Operator) syncResolvingNamespace(obj interface{}) error { return err } updatedSubs = o.setIPReference(updatedSubs, maxGeneration+1, installPlanReference) - for _, updatedSub := range updatedSubs { - for i, sub := range subs { - if sub.Name == updatedSub.Name && sub.Namespace == updatedSub.Namespace { - subs[i] = updatedSub - } - } - } } else { logger.Debugf("no subscriptions were updated") } + // Remove resolutionfailed condition from subscriptions + subs = o.removeSubsCond(subs, v1alpha1.SubscriptionResolutionFailed) + newSub := true + for _, updatedSub := range updatedSubs { + updatedSub.Status.RemoveConditions(v1alpha1.SubscriptionResolutionFailed) + for i, sub := range subs { + if sub.Name == updatedSub.Name && sub.Namespace == updatedSub.Namespace { + subs[i] = updatedSub + newSub = false + break + } + } + if newSub { + subs = append(subs, updatedSub) + continue + } + newSub = true + } + + // Update subscriptions with all changes so far + _, updateErr := o.updateSubscriptionStatuses(subs) + if updateErr != nil { + logger.WithError(updateErr).Warn("failed to update subscription conditions") + return updateErr + } + return nil } @@ -1043,11 +1065,6 @@ func (o *Operator) syncSubscriptions(obj interface{}) error { } func (o *Operator) nothingToUpdate(logger *logrus.Entry, sub *v1alpha1.Subscription) bool { - // Only sync if catalog has been updated since last sync time - if o.sourcesLastUpdate.Before(sub.Status.LastUpdated.Time) && sub.Status.State != v1alpha1.SubscriptionStateNone && sub.Status.State != v1alpha1.SubscriptionStateUpgradeAvailable { - logger.Debugf("skipping update: no new updates to catalog since last sync at %s", sub.Status.LastUpdated.String()) - return true - } if sub.Status.InstallPlanRef != nil && sub.Status.State == v1alpha1.SubscriptionStateUpgradePending { logger.Debugf("skipping update: installplan already created") return true @@ -1088,12 +1105,7 @@ func (o *Operator) ensureSubscriptionInstallPlanState(logger *logrus.Entry, sub out.Status.CurrentCSV = out.Spec.StartingCSV out.Status.LastUpdated = o.now() - updated, err := o.client.OperatorsV1alpha1().Subscriptions(sub.GetNamespace()).UpdateStatus(context.TODO(), out, metav1.UpdateOptions{}) - if err != nil { - return nil, false, err - } - - return updated, true, nil + return out, true, nil } func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha1.Subscription, querier resolver.SourceQuerier) (*v1alpha1.Subscription, bool, error) { @@ -1101,7 +1113,7 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha return sub, false, nil } - csv, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(sub.GetNamespace()).Get(context.TODO(), sub.Status.CurrentCSV, metav1.GetOptions{}) + _, err := o.client.OperatorsV1alpha1().ClusterServiceVersions(sub.GetNamespace()).Get(context.TODO(), sub.Status.CurrentCSV, metav1.GetOptions{}) out := sub.DeepCopy() if err != nil { logger.WithError(err).WithField("currentCSV", sub.Status.CurrentCSV).Debug("error fetching csv listed in subscription status") @@ -1111,14 +1123,7 @@ func (o *Operator) ensureSubscriptionCSVState(logger *logrus.Entry, sub *v1alpha if err := querier.Queryable(); err != nil { return nil, false, err } - b, _, _ := querier.FindReplacement(&csv.Spec.Version.Version, sub.Status.CurrentCSV, sub.Spec.Package, sub.Spec.Channel, registry.CatalogKey{Name: sub.Spec.CatalogSource, Namespace: sub.Spec.CatalogSourceNamespace}) - if b != nil { - o.logger.Tracef("replacement %s bundle found for current bundle %s", b.CsvName, sub.Status.CurrentCSV) - out.Status.State = v1alpha1.SubscriptionStateUpgradeAvailable - } else { - out.Status.State = v1alpha1.SubscriptionStateAtLatest - } - + out.Status.State = v1alpha1.SubscriptionStateAtLatest out.Status.InstalledCSV = sub.Status.CurrentCSV } @@ -1236,23 +1241,45 @@ func (o *Operator) createInstallPlan(namespace string, gen int, subs []*v1alpha1 return reference.GetReference(res) } -func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType, reason, message string, setTrue bool) []*v1alpha1.Subscription { +// setSubsCond will set the condition to the subscription if it doesn't already +// exist or if it is different +// Only return the list of updated subscriptions +func (o *Operator) setSubsCond(subs []*v1alpha1.Subscription, cond v1alpha1.SubscriptionCondition) []*v1alpha1.Subscription { var ( lastUpdated = o.now() + subList []*v1alpha1.Subscription ) + for _, sub := range subs { + subCond := sub.Status.GetCondition(cond.Type) + if subCond.Equals(cond) { + continue + } sub.Status.LastUpdated = lastUpdated + sub.Status.SetCondition(cond) + subList = append(subList, sub) + } + return subList +} + +// removeSubsCond will remove the condition to the subscription if it exists +// Only return the list of updated subscriptions +func (o *Operator) removeSubsCond(subs []*v1alpha1.Subscription, condType v1alpha1.SubscriptionConditionType) []*v1alpha1.Subscription { + var ( + lastUpdated = o.now() + ) + var subList []*v1alpha1.Subscription + for _, sub := range subs { cond := sub.Status.GetCondition(condType) - cond.Reason = reason - cond.Message = message - if setTrue { - cond.Status = corev1.ConditionTrue - } else { - cond.Status = corev1.ConditionFalse + // if status is ConditionUnknown, the condition doesn't exist. Just skip + if cond.Status == corev1.ConditionUnknown { + continue } - sub.Status.SetCondition(cond) + sub.Status.LastUpdated = lastUpdated + sub.Status.RemoveConditions(condType) + subList = append(subList, sub) } - return subs + return subList } func (o *Operator) updateSubscriptionStatuses(subs []*v1alpha1.Subscription) ([]*v1alpha1.Subscription, error) { diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go index 7acbc8a8a3..121c2563f4 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/querier.go @@ -31,6 +31,7 @@ type SourceQuerier interface { FindProvider(api opregistry.APIKey, initialSource registry.CatalogKey, excludedPackages map[string]struct{}) (*api.Bundle, *registry.CatalogKey, error) FindBundle(pkgName, channelName, bundleName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) FindLatestBundle(pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) + // Deprecated: This FindReplacement function will be deprecated soon FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) Queryable() error } @@ -123,6 +124,7 @@ func (q *NamespaceSourceQuerier) FindLatestBundle(pkgName, channelName string, i return nil, nil, fmt.Errorf("%s/%s not found in any available CatalogSource", pkgName, channelName) } +// Deprecated: This FindReplacement function will be deprecated soon func (q *NamespaceSourceQuerier) FindReplacement(currentVersion *semver.Version, bundleName, pkgName, channelName string, initialSource registry.CatalogKey) (*api.Bundle, *registry.CatalogKey, error) { errs := []error{}