Skip to content

Commit

Permalink
Improve manifest apply and delete (openshift-knative#606)
Browse files Browse the repository at this point in the history
* Improve manifest apply and delete

* Introduce roleOrRoleBinding var
  • Loading branch information
aliok authored Oct 22, 2020
1 parent 3c74b7a commit 0557b8a
Showing 1 changed file with 32 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ const (
finalizerName = "knative-kafka-openshift"
)

var log = logf.Log.WithName("controller_knativekafka")
var (
log = logf.Log.WithName("controller_knativekafka")
role = mf.Any(mf.ByKind("ClusterRole"), mf.ByKind("Role"))
rolebinding = mf.Any(mf.ByKind("ClusterRoleBinding"), mf.ByKind("RoleBinding"))
roleOrRoleBinding = mf.Any(role, rolebinding)
)

type stage func(*mf.Manifest, *operatorv1alpha1.KnativeKafka) error

Expand Down Expand Up @@ -227,9 +232,20 @@ func (r *ReconcileKnativeKafka) transform(manifest *mf.Manifest, instance *opera
// Install Knative Kafka components
func (r *ReconcileKnativeKafka) apply(manifest *mf.Manifest, instance *operatorv1alpha1.KnativeKafka) error {
log.Info("Installing manifest")
if err := manifest.Apply(); err != nil {
// The Operator needs a higher level of permissions if it 'bind's non-existent roles.
// To avoid this, we strictly order the manifest application as (Cluster)Roles, then
// (Cluster)RoleBindings, then the rest of the manifest.
if err := manifest.Filter(role).Apply(); err != nil {
instance.Status.MarkInstallFailed(err.Error())
return fmt.Errorf("failed to apply (cluster)roles in manifest: %w", err)
}
if err := manifest.Filter(rolebinding).Apply(); err != nil {
instance.Status.MarkInstallFailed(err.Error())
return fmt.Errorf("failed to apply (cluster)rolebindings in manifest: %w", err)
}
if err := manifest.Filter(not(roleOrRoleBinding)).Apply(); err != nil {
instance.Status.MarkInstallFailed(err.Error())
return fmt.Errorf("failed to apply manifest: %w", err)
return fmt.Errorf("failed to apply non rbac manifest: %w", err)
}
instance.Status.MarkInstallSucceeded()
return nil
Expand Down Expand Up @@ -265,11 +281,13 @@ func (r *ReconcileKnativeKafka) deleteResources(manifest *mf.Manifest, instance
return nil
}
log.Info("Deleting resources in manifest")
if err := manifest.Delete(); err != nil {
// TODO: any conditions?
return fmt.Errorf("failed to apply manifest: %w", err)
if err := manifest.Filter(mf.NoCRDs, not(roleOrRoleBinding)).Delete(); err != nil {
return fmt.Errorf("failed to remove non-crd/non-rbac resources: %w", err)
}
// Delete Roles last, as they may be useful for human operators to clean up.
if err := manifest.Filter(roleOrRoleBinding).Delete(); err != nil {
return fmt.Errorf("failed to remove rbac: %w", err)
}
// TODO: any conditions?
return nil
}

Expand Down Expand Up @@ -379,3 +397,10 @@ func executeStages(instance *operatorv1alpha1.KnativeKafka, manifest *mf.Manifes
}
return nil
}

// TODO: get rid of this when we update to Manifestival version that has this function
var not = func(pred mf.Predicate) mf.Predicate {
return func(u *unstructured.Unstructured) bool {
return !pred(u)
}
}

0 comments on commit 0557b8a

Please sign in to comment.