Do more to record bootstrap module status

 - adapt the code for tracking assemblages and recording sync states
   and module summaries, from module_controller to
   bootstrapmodule_controller
 - use KustomizeReadyState from pkg/api/, consistently
 - try to get the RBAC annotations more accurate (again)

Signed-off-by: Michael Bridgen <mikeb@squaremobius.net>

assemblage/controllers/assemblage_controller.go

@@ -9,10 +9,7 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/go-logr/logr"
-	apimeta "k8s.io/apimachinery/pkg/api/meta"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -122,7 +119,7 @@ func (r *AssemblageReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 			if syncStatus.State == "" {
 				switch op {
 				case controllerutil.OperationResultNone:
-					syncStatus.State = readyState(&kustom)
+					syncStatus.State = syncapi.KustomizeReadyState(&kustom)
 				default:
 					syncStatus.State = syncapi.StateUpdating
 				}
@@ -141,25 +138,6 @@ func (r *AssemblageReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	return ctrl.Result{}, nil
 }
 
-func readyState(obj meta.ObjectWithStatusConditions) syncapi.SyncState {
-	conditions := obj.GetStatusConditions()
-	c := apimeta.FindStatusCondition(*conditions, meta.ReadyCondition)
-	switch {
-	case c == nil:
-		return syncapi.StateUpdating
-	case c.Status == metav1.ConditionTrue:
-		return syncapi.StateSucceeded
-	case c.Status == metav1.ConditionFalse:
-		if c.Reason == meta.ReconciliationFailedReason {
-			return syncapi.StateFailed
-		} else {
-			return syncapi.StateUpdating
-		}
-	default: // FIXME possibly StateUnknown?
-		return syncapi.StateUpdating
-	}
-}
-
 func makeBindingFunc(ctx context.Context, log logr.Logger, namespacedClient client.Client, bindings []syncapi.Binding, stack []string) func(string) string {
 	memo := map[string]string{}
 	return func(name string) string {

assemblage/go.mod

@@ -4,12 +4,11 @@ go 1.15
 
 require (
 	github.com/fluxcd/kustomize-controller/api v0.12.0
-	github.com/fluxcd/pkg/apis/meta v0.9.0
 	github.com/fluxcd/source-controller/api v0.12.2
 	github.com/go-logr/logr v0.4.0
 	github.com/onsi/ginkgo v1.14.1
 	github.com/onsi/gomega v1.10.2
-	github.com/squaremo/fleeet/pkg v0.0.2
+	github.com/squaremo/fleeet/pkg v0.0.3-rc1
 	k8s.io/api v0.20.4
 	k8s.io/apimachinery v0.21.0
 	k8s.io/client-go v0.20.4

assemblage/go.sum

@@ -373,8 +373,8 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
 github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
 github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
 github.com/spf13/viper v1.7.0/go.mod h1:8WkrPz2fc9jxqZNCJI/76HCieCp4Q8HaLFoCha5qpdg=
-github.com/squaremo/fleeet/pkg v0.0.2 h1:yMfRYspuDb4S/qdXJ+J1rZjyBLpeNqglRbtXfOseeoM=
-github.com/squaremo/fleeet/pkg v0.0.2/go.mod h1:zmOneM0CcQv4RdOgsOCMVzVIlwt0yEGykPvpQqfSG8s=
+github.com/squaremo/fleeet/pkg v0.0.3-rc1 h1:IVlqw29VOBly03IyRiuUxvNBreFToxlYN0iYbiZwtUc=
+github.com/squaremo/fleeet/pkg v0.0.3-rc1/go.mod h1:zmOneM0CcQv4RdOgsOCMVzVIlwt0yEGykPvpQqfSG8s=
 github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag1KpM8ahLw8=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=

module/api/v1alpha1/bootstrapmodule_types.go

@@ -10,6 +10,8 @@ import (
 	syncapi "github.com/squaremo/fleeet/pkg/api"
 )
 
+const KindBootstrapModule = "BootstrapModule"
+
 // BootstrapModuleSpec defines the desired state of BootstrapModule
 type BootstrapModuleSpec struct {
 	// Selector gives the criteria for assigning this module to a

module/api/v1alpha1/remoteassemblage_types.go

@@ -63,7 +63,12 @@ type LocalKubeconfigReference struct {
 
 // RemoteAssemblageStatus defines the observed state of RemoteAssemblage
 type RemoteAssemblageStatus struct {
-	Syncs []syncapi.SyncStatus `json:"syncs,omitempty"`
+	Syncs []SyncStatus `json:"syncs,omitempty"`
+}
+
+type SyncStatus struct {
+	Name  string            `json:"name"`
+	State syncapi.SyncState `json:"state"`
 }
 
 //+kubebuilder:object:root=true

module/config/crd/bases/fleet.squaremo.dev_remoteassemblages.yaml

@@ -149,112 +149,14 @@ spec:
             properties:
               syncs:
                 items:
-                  description: SyncStatus gives the status of a specific sync.
                   properties:
+                    name:
+                      type: string
                     state:
-                      description: State gives the outcome of last applied sync spec.
                       type: string
-                    sync:
-                      description: Sync gives the last applied sync spec.
-                      properties:
-                        bindings:
-                          description: Bindings gives a list of variable bindings
-                            to use when evaluating the package spec in the sync
-                          items:
-                            description: Binding specifies how to obtain a value to
-                              bind to a name. The name can then be mentioned elsewhere
-                              in an object, and be replaced with the value as evaluated.
-                            properties:
-                              name:
-                                type: string
-                              objectFieldRef:
-                                properties:
-                                  apiVersion:
-                                    description: APIVersion gives the APIVersion (<group>/<version>)
-                                      for the object's type
-                                    type: string
-                                  fieldPath:
-                                    description: Path is a JSONPointer expression
-                                      for finding the value in the object identified
-                                    type: string
-                                  kind:
-                                    description: Kind gives the kind of the object's
-                                      type
-                                    type: string
-                                  name:
-                                    description: Name names the object
-                                    type: string
-                                required:
-                                - fieldPath
-                                - kind
-                                - name
-                                type: object
-                              value:
-                                type: string
-                            required:
-                            - name
-                            type: object
-                          type: array
-                        name:
-                          description: Name gives the sync a name so it can be correlated
-                            to the status
-                          type: string
-                        package:
-                          default:
-                            kustomize:
-                              path: .
-                          description: Package defines how to deal with the configuration
-                            at the source, e.g., if it's a kustomization (or YAML
-                            files)
-                          properties:
-                            kustomize:
-                              properties:
-                                path:
-                                  default: .
-                                  description: Path gives the path within the source
-                                    to treat as the Kustomization root.
-                                  type: string
-                                substitute:
-                                  additionalProperties:
-                                    type: string
-                                  description: Substitute gives a map of names to
-                                    values to substitute in the YAML built from the
-                                    kustomization.
-                                  type: object
-                              type: object
-                          type: object
-                        source:
-                          description: Source gives the specification for how to get
-                            the configuration to be synced
-                          properties:
-                            git:
-                              properties:
-                                url:
-                                  description: URL gives the URL for the git repository
-                                  type: string
-                                version:
-                                  description: Version gives either the revision or
-                                    tag at which to get the git repo
-                                  properties:
-                                    revision:
-                                      type: string
-                                    tag:
-                                      type: string
-                                  type: object
-                              required:
-                              - url
-                              - version
-                              type: object
-                          required:
-                          - git
-                          type: object
-                      required:
-                      - name
-                      - source
-                      type: object
                   required:
+                  - name
                   - state
-                  - sync
                   type: object
                 type: array
             type: object

module/config/rbac/role.yaml

@@ -21,6 +21,7 @@ rules:
   verbs:
   - get
   - list
+  - watch
 - apiGroups:
   - fleet.squaremo.dev
   resources:

module/controllers/bootstrapmodule_controller.go

@@ -39,6 +39,11 @@ type BootstrapModuleReconciler struct {
 //+kubebuilder:rbac:groups=fleet.squaremo.dev,resources=bootstrapmodules/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=fleet.squaremo.dev,resources=bootstrapmodules/finalizers,verbs=update
 
+// These are what the controller creates:
+//+kubebuilder:rbac:groups=fleet.squaremo.dev,resources=remoteassemblages,verbs=get;list;watch;create;update;patch;delete
+//+kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories,verbs=get;list;watch;create;update;patch;delete
+
+// The controller watches these, to see when the selection changes
 //+kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters,verbs=get;list;watch
 
 // Reconcile moves the cluster closer to the desired state, for a particular
@@ -94,7 +99,18 @@ func (r *BootstrapModuleReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		Kind:       source.Kind,
 	}
 
+	summary := &fleetv1.SyncSummary{}
+
+	// Keep track of the assemblages which did require this module;
+	// afterwards, this will be helpful to determine the assemblages
+	// which need the module removed.
+	requiredAsm := map[string]struct{}{}
+
+clusters:
 	for _, cluster := range clusters.Items {
+		summary.Total++
+		requiredAsm[cluster.GetName()] = struct{}{}
+
 		asm := &fleetv1.RemoteAssemblage{}
 		asm.Namespace = cluster.GetNamespace()
 		asm.Name = cluster.GetName()
@@ -136,19 +152,96 @@ func (r *BootstrapModuleReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 			return nil
 		})
 		if err != nil {
-			return ctrl.Result{}, err
+			summary.Failed++
+			log.Error(err, "updating remote assemblages", "assemblage", asm.Name)
+		} else {
+			log.V(1).Info("created/updated RemoteAssemblage", "name", asm.Name, "operation", op)
+			for _, sync := range asm.Status.Syncs {
+				if sync.Name == mod.Name {
+					switch sync.State {
+					case syncapi.StateSucceeded:
+						summary.Succeeded++
+					case syncapi.StateFailed:
+						summary.Failed++
+					default:
+						summary.Updating++
+					}
+					continue clusters // all done here
+				}
+				// no change made, but status not found -> updating
+				summary.Updating++
+			}
+		}
+	}
+
+	// Find all assemblages indexed as owned by (i.e., including) this
+	// module
+	var asms fleetv1.RemoteAssemblageList
+	if err := r.List(ctx, &asms, client.InNamespace(req.Namespace), client.MatchingFields{remoteAssemblageOwnerKey: req.Name}); err != nil {
+		return ctrl.Result{}, fmt.Errorf("listing assemblages for this module: %w", err)
+	}
+
+	// This loop removes the bootstrap module from any assemblage for a cluster that _wasn't_
+	// selected. (Remember, these assemblages were selected because they were owned by this module,
+	// implying that at some point the module was assigned to the cluster)
+	for _, asm := range asms.Items {
+		if _, ok := requiredAsm[asm.GetName()]; !ok {
+			syncs := asm.Spec.Syncs
+			for i, sync := range syncs {
+				if sync.Name == mod.Name {
+					if _, err := controllerutil.CreateOrPatch(ctx, r.Client, &asm, func() error {
+						asm.Spec.Syncs = append(syncs[:i], syncs[i+1:]...)
+						removeOwnerRef(&mod, &asm)
+						return nil
+					}); err != nil {
+						log.Error(err, "removing module from remote assemblage", "assemblage", asm.Name)
+					}
+					// FIXME: can this `break` from the loop at this point?
+				}
+			}
 		}
-		log.V(1).Info("created/updated RemoteAssemblage", "name", asm.Name, "operation", op)
 	}
-	// TODO find any redundant sources and assemblages and delete them
+
+	mod.Status.ObservedSync = &mod.Spec.Sync
+	mod.Status.Summary = summary
+	if err := r.Status().Update(ctx, &mod); err != nil {
+		return ctrl.Result{}, fmt.Errorf("updating status of bootstrap module: %w", err)
+	}
 
 	return ctrl.Result{}, nil
 }
 
+const remoteAssemblageOwnerKey = "ownerBootstrapModule"
+
 // SetupWithManager sets up the controller with the Manager.
 func (r *BootstrapModuleReconciler) SetupWithManager(mgr ctrl.Manager) error {
+	// This index is to conveniently find the remote assemblages that a bootstrap module owns (i.e.,
+	// is included in). The watch on RemoteAssemblages below will enqueue each bootstrap module
+	// owner of a remote aseemblage that has been updated.
+	if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &fleetv1.RemoteAssemblage{}, remoteAssemblageOwnerKey, func(obj client.Object) []string {
+		asm := obj.(*fleetv1.RemoteAssemblage)
+		var moduleOwners []string
+		for _, owner := range asm.GetOwnerReferences() {
+			// FIXME: make this more reliable? What are the consequences of getting another API's
+			// BootstrapModule mixed in here? Something like this might be better:
+			// https://github.com/kubernetes-sigs/controller-runtime/blob/v0.11.1/pkg/handler/enqueue_owner.go#L46
+			if owner.Kind == fleetv1.KindBootstrapModule {
+				moduleOwners = append(moduleOwners, owner.Name)
+			}
+		}
+		return moduleOwners
+	}); err != nil {
+		return err
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&fleetv1.BootstrapModule{}).
+		Watches(
+			&source.Kind{Type: &fleetv1.RemoteAssemblage{}},
+			&handler.EnqueueRequestForOwner{
+				OwnerType:    &fleetv1.BootstrapModule{},
+				IsController: false,
+			}).
 
 		// Enqueue all the BootstrapModule objects that pertain to a
 		// particular cluster