Skip to content

Commit

Permalink
(bug) CSV name change causes upgrade to fail
Browse files Browse the repository at this point in the history
If a head CSV has a different name than the CSV it replaces, upgrade
fails because olm fails to transfer ownership of the APIService from
the previous CSV to the head CSV.

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1723818
  • Loading branch information
tkashem committed Jun 28, 2019
1 parent 6bf64d0 commit d3c2b32
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 35 deletions.
88 changes: 65 additions & 23 deletions pkg/controller/operators/olm/apiservices.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package olm

import (
"errors"
"fmt"
"strings"
"time"
Expand Down Expand Up @@ -65,18 +66,6 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion,
})

errs := []error{}
owners := []ownerutil.Owner{csv}

// Get replacing CSV if exists
replacing, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(csv.GetNamespace()).Get(csv.Spec.Replaces)
if err != nil && !k8serrors.IsNotFound(err) {
logger.WithError(err).Warn("could not get replacement csv")
return err
}
if replacing != nil {
owners = append(owners, replacing)
}

ruleChecker := install.NewCSVRuleChecker(a.lister.RbacV1().RoleLister(), a.lister.RbacV1().RoleBindingLister(), a.lister.RbacV1().ClusterRoleLister(), a.lister.RbacV1().ClusterRoleBindingLister(), csv)
for _, desc := range csv.GetOwnedAPIServiceDescriptions() {
apiServiceName := desc.GetName()
Expand All @@ -92,8 +81,15 @@ func (a *Operator) checkAPIServiceResources(csv *v1alpha1.ClusterServiceVersion,
}

// Check if the APIService is adoptable
if !ownerutil.AdoptableLabels(apiService.GetLabels(), true, owners...) {
logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Debug("adoption failed")
adoptable, err := a.isAPIServiceAdoptable(csv, apiService)
if err != nil {
logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Errorf("adoption check failed - %v", err)
errs = append(errs, err)
return utilerrors.NewAggregate(errs)
}

if !adoptable {
logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Errorf("adoption failed")
err := olmerrors.NewUnadoptableError("", apiServiceName)
logger.WithError(err).Warn("found unadoptable apiservice")
errs = append(errs, err)
Expand Down Expand Up @@ -707,17 +703,12 @@ func (a *Operator) installAPIServiceRequirements(desc v1alpha1.APIServiceDescrip
}
apiService.SetName(apiServiceName)
} else {
owners := []ownerutil.Owner{csv}

// Get replacing CSV
replaces, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(csv.GetNamespace()).Get(csv.Spec.Replaces)
if err == nil {
owners = append(owners, replaces)
adoptable, err := a.isAPIServiceAdoptable(csv, apiService)
if err != nil {
logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Errorf("adoption check failed - %v", err)
}

// check if the APIService is adoptable
if !ownerutil.AdoptableLabels(apiService.GetLabels(), true, owners...) {
logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Debug("adoption failed")
if !adoptable{
return nil, fmt.Errorf("pre-existing APIService %s is not adoptable", apiServiceName)
}
}
Expand Down Expand Up @@ -759,3 +750,54 @@ func APIServiceNameToServiceName(apiServiceName string) string {
// Replace all '.'s with "-"s to convert to a DNS-1035 label
return strings.Replace(apiServiceName, ".", "-", -1)
}

func (a *Operator) isAPIServiceAdoptable(target *v1alpha1.ClusterServiceVersion, apiService *apiregistrationv1.APIService) (adoptable bool, err error) {
if apiService == nil || target == nil {
err = errors.New("invalid input")
return
}

labels := apiService.GetLabels()
ownerKind := labels[ownerutil.OwnerKind]
ownerName := labels[ownerutil.OwnerKey]
ownerNamespace := labels[ownerutil.OwnerNamespaceKey]

if ownerKind == "" || ownerNamespace == "" || ownerName == "" {
return
}

if err := ownerutil.InferGroupVersionKind(target); err != nil {
a.logger.Warn(err.Error())
}

targetKind := target.GetObjectKind().GroupVersionKind().Kind
if ownerKind != targetKind {
return
}

// Get the CSV that target replaces
replacing, replaceGetErr := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(target.GetNamespace()).Get(target.Spec.Replaces)
if replaceGetErr != nil && !k8serrors.IsNotFound(replaceGetErr) && !k8serrors.IsGone(replaceGetErr) {
err = replaceGetErr
return
}

// Get the current owner CSV of the APIService
currentOwnerCSV, ownerGetErr := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(ownerNamespace).Get(ownerName)
if ownerGetErr != nil && !k8serrors.IsNotFound(ownerGetErr) && !k8serrors.IsGone(ownerGetErr) {
err = ownerGetErr
return
}

owners := []ownerutil.Owner{target}
if replacing != nil {
owners = append(owners, replacing)
}
if currentOwnerCSV != nil && (
currentOwnerCSV.Status.Phase == v1alpha1.CSVPhaseReplacing || currentOwnerCSV.Status.Phase == v1alpha1.CSVPhaseDeleting) {
owners = append(owners, currentOwnerCSV)
}

adoptable = ownerutil.AdoptableLabels(apiService.GetLabels(), true, owners...)
return
}
19 changes: 7 additions & 12 deletions pkg/controller/operators/olm/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"strings"
"time"

log "github.com/sirupsen/logrus"
v1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1"
"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1386,17 +1387,6 @@ func (a *Operator) getReplacementChain(in *v1alpha1.ClusterServiceVersion, csvsI
}

func (a *Operator) apiServiceOwnerConflicts(csv *v1alpha1.ClusterServiceVersion) error {
// Get replacing CSV if exists
replacing, err := a.lister.OperatorsV1alpha1().ClusterServiceVersionLister().ClusterServiceVersions(csv.GetNamespace()).Get(csv.Spec.Replaces)
if err != nil && !k8serrors.IsNotFound(err) && !k8serrors.IsGone(err) {
return err
}

owners := []ownerutil.Owner{csv}
if replacing != nil {
owners = append(owners, replacing)
}

for _, desc := range csv.GetOwnedAPIServiceDescriptions() {
// Check if the APIService exists
apiService, err := a.lister.APIRegistrationV1().APIServiceLister().Get(desc.GetName())
Expand All @@ -1408,7 +1398,12 @@ func (a *Operator) apiServiceOwnerConflicts(csv *v1alpha1.ClusterServiceVersion)
continue
}

if !ownerutil.AdoptableLabels(apiService.GetLabels(), true, owners...) {
adoptable, err := a.isAPIServiceAdoptable(csv, apiService)
if err != nil {
a.logger.WithFields(log.Fields{"obj": "apiService", "labels": apiService.GetLabels()}).Errorf("adoption check failed - %v", err)
}

if !adoptable {
return ErrAPIServiceOwnerConflict
}
}
Expand Down

0 comments on commit d3c2b32

Please sign in to comment.