From 2c097c1621935f46718dfff0a510f83536f69d8a Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Mon, 6 Mar 2023 20:05:26 -0600 Subject: [PATCH] rollouts: support reposync option (#3850) --- rollouts/api/v1alpha1/remoterootsync_types.go | 9 +- .../bases/gitops.kpt.dev_remoterootsyncs.yaml | 5 ++ .../controllers/remoterootsync_controller.go | 84 +++++++++++++++---- rollouts/controllers/rollout_controller.go | 48 ++++++----- rollouts/controllers/status.go | 22 +++-- 5 files changed, 119 insertions(+), 49 deletions(-) diff --git a/rollouts/api/v1alpha1/remoterootsync_types.go b/rollouts/api/v1alpha1/remoterootsync_types.go index 54f229577c..79b2f196bc 100644 --- a/rollouts/api/v1alpha1/remoterootsync_types.go +++ b/rollouts/api/v1alpha1/remoterootsync_types.go @@ -25,11 +25,10 @@ import ( // RemoteRootSyncSpec defines the desired state of RemoteRootSync type RemoteRootSyncSpec struct { - // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster - // Important: Run "make" to regenerate code after modifying this file - - ClusterRef ClusterRef `json:"clusterRef,omitempty"` - Template *RootSyncInfo `json:"template,omitempty"` + // ClusterReference contains the identify information need to refer a cluster. + ClusterRef ClusterRef `json:"clusterRef,omitempty"` + Template *RootSyncInfo `json:"template,omitempty"` + Type SyncTemplateType `json:"type,omitempty"` } type RootSyncInfo struct { diff --git a/rollouts/config/crd/bases/gitops.kpt.dev_remoterootsyncs.yaml b/rollouts/config/crd/bases/gitops.kpt.dev_remoterootsyncs.yaml index eaeb68f6fd..43fe33a1a9 100644 --- a/rollouts/config/crd/bases/gitops.kpt.dev_remoterootsyncs.yaml +++ b/rollouts/config/crd/bases/gitops.kpt.dev_remoterootsyncs.yaml @@ -111,6 +111,11 @@ spec: type: string type: object type: object + type: + enum: + - RootSync + - RepoSync + type: string type: object status: description: RemoteRootSyncStatus defines the observed state of RemoteRootSync diff --git a/rollouts/controllers/remoterootsync_controller.go b/rollouts/controllers/remoterootsync_controller.go index db08c19bcb..caaa3a0daf 100644 --- a/rollouts/controllers/remoterootsync_controller.go +++ b/rollouts/controllers/remoterootsync_controller.go @@ -46,8 +46,13 @@ import ( ) var ( + // The RootSync object always gets put in the config-management-system namespace, + // while the RepoSync object will get its namespace from the RemoteRootSync object's + // metadata.namespace. + // See examples at: https://cloud.google.com/anthos-config-management/docs/how-to/multiple-repositories rootSyncNamespace = "config-management-system" - rootSyncGVK = schema.GroupVersionKind{ + + rootSyncGVK = schema.GroupVersionKind{ Group: "configsync.gke.io", Version: "v1beta1", Kind: "RootSync", @@ -58,6 +63,17 @@ var ( Resource: "rootsyncs", } + repoSyncGVK = schema.GroupVersionKind{ + Group: "configsync.gke.io", + Version: "v1beta1", + Kind: "RepoSync", + } + repoSyncGVR = schema.GroupVersionResource{ + Group: "configsync.gke.io", + Version: "v1beta1", + Resource: "reposyncs", + } + remoteRootSyncNameLabel = "gitops.kpt.dev/remoterootsync-name" remoteRootSyncNamespaceLabel = "gitops.kpt.dev/remoterootsync-namespace" ) @@ -167,7 +183,6 @@ func (r *RemoteRootSyncReconciler) Reconcile(ctx context.Context, req ctrl.Reque } func (r *RemoteRootSyncReconciler) syncExternalSync(ctx context.Context, rrs *gitopsv1alpha1.RemoteRootSync) (string, error) { - syncName := rrs.Name clusterRef := &rrs.Spec.ClusterRef dynCl, err := r.getDynamicClientForCluster(ctx, clusterRef) @@ -175,15 +190,15 @@ func (r *RemoteRootSyncReconciler) syncExternalSync(ctx context.Context, rrs *gi return "", fmt.Errorf("failed to create client: %w", err) } - if err := r.patchRootSync(ctx, dynCl, syncName, rrs); err != nil { + if err := r.patchExternalSync(ctx, dynCl, rrs); err != nil { return "", fmt.Errorf("failed to create/update sync: %w", err) } r.setupWatches(ctx, rrs.Name, rrs.Namespace, rrs.Spec.ClusterRef) - syncStatus, err := checkSyncStatus(ctx, dynCl, syncName) + syncStatus, err := checkSyncStatus(ctx, dynCl, rrs) if err != nil { - return "", fmt.Errorf("faild to check status: %w", err) + return "", fmt.Errorf("failed to check status: %w", err) } return syncStatus, nil @@ -225,25 +240,33 @@ func (r *RemoteRootSyncReconciler) updateStatus(ctx context.Context, rrs *gitops return r.Client.Status().Update(ctx, rrs) } -// patchRootSync patches the RootSync in the remote clusters targeted by +// patchExternalSync patches the external sync in the remote clusters targeted by // the clusterRefs based on the latest revision of the template in the RemoteRootSync. -func (r *RemoteRootSyncReconciler) patchRootSync(ctx context.Context, client dynamic.Interface, name string, rrs *gitopsv1alpha1.RemoteRootSync) error { +func (r *RemoteRootSyncReconciler) patchExternalSync(ctx context.Context, client dynamic.Interface, rrs *gitopsv1alpha1.RemoteRootSync) error { logger := klog.FromContext(ctx) - newRootSync, err := BuildObjectsToApply(rrs) + gvr, gvk, err := getGvrAndGvk(rrs.Spec.Type) + if err != nil { + return err + } + + namespace := getExternalSyncNamespace(rrs) + + newRootSync, err := BuildObjectsToApply(rrs, gvk, namespace) if err != nil { return err } data, err := json.Marshal(newRootSync) if err != nil { - return fmt.Errorf("failed to encode root sync to JSON: %w", err) + return fmt.Errorf("failed to encode %s to JSON: %w", gvk.Kind, err) } - _, err = client.Resource(rootSyncGVR).Namespace(rootSyncNamespace).Patch(ctx, name, types.ApplyPatchType, data, metav1.PatchOptions{FieldManager: name}) + + _, err = client.Resource(gvr).Namespace(namespace).Patch(ctx, rrs.Name, types.ApplyPatchType, data, metav1.PatchOptions{FieldManager: rrs.Name}) if err != nil { - return fmt.Errorf("failed to patch RootSync: %w", err) + return fmt.Errorf("failed to patch %s: %w", gvk.Kind, err) } - logger.Info("RootSync resource created/updated", "rootSync", klog.KRef(rootSyncNamespace, name)) + logger.Info(fmt.Sprintf("%s resource created/updated", gvk.Kind), gvr.Resource, klog.KRef(namespace, rrs.Name)) return nil } @@ -313,17 +336,20 @@ func (r *RemoteRootSyncReconciler) pruneWatches(rrsnn types.NamespacedName, clus } } -// BuildObjectsToApply config root sync -func BuildObjectsToApply(remoterootsync *gitopsv1alpha1.RemoteRootSync) (*unstructured.Unstructured, error) { +// BuildObjectsToApply configures the external sync +func BuildObjectsToApply(remoterootsync *gitopsv1alpha1.RemoteRootSync, + gvk schema.GroupVersionKind, + namespace string) (*unstructured.Unstructured, error) { + newRootSync, err := runtime.DefaultUnstructuredConverter.ToUnstructured(remoterootsync.Spec.Template) if err != nil { return nil, fmt.Errorf("failed to convert to unstructured type: %w", err) } u := unstructured.Unstructured{Object: newRootSync} - u.SetGroupVersionKind(rootSyncGVK) + u.SetGroupVersionKind(gvk) u.SetName(remoterootsync.Name) - u.SetNamespace(rootSyncNamespace) + u.SetNamespace(namespace) labels := u.GetLabels() if labels == nil { @@ -345,8 +371,13 @@ func (r *RemoteRootSyncReconciler) deleteExternalResources(ctx context.Context, return err } + gvr, _, err := getGvrAndGvk(remoterootsync.Spec.Type) + if err != nil { + return err + } + logger.Info("Deleting external resource") - err = dynCl.Resource(rootSyncGVR).Namespace("config-management-system").Delete(ctx, remoterootsync.Name, metav1.DeleteOptions{}) + err = dynCl.Resource(gvr).Namespace(getExternalSyncNamespace(remoterootsync)).Delete(ctx, remoterootsync.Name, metav1.DeleteOptions{}) if err != nil && !apierrors.IsNotFound(err) { return err } @@ -368,6 +399,25 @@ func (r *RemoteRootSyncReconciler) getDynamicClientForCluster(ctx context.Contex return dynamicClient, nil } +func getGvrAndGvk(t gitopsv1alpha1.SyncTemplateType) (schema.GroupVersionResource, schema.GroupVersionKind, error) { + switch t { + case gitopsv1alpha1.TemplateTypeRootSync, "": + return rootSyncGVR, rootSyncGVK, nil + case gitopsv1alpha1.TemplateTypeRepoSync: + return repoSyncGVR, repoSyncGVK, nil + default: + return schema.GroupVersionResource{}, schema.GroupVersionKind{}, fmt.Errorf("invalid sync type %q", t) + } +} + +func getExternalSyncNamespace(rrs *gitopsv1alpha1.RemoteRootSync) string { + if rrs.Spec.Type == gitopsv1alpha1.TemplateTypeRepoSync { + return rrs.Namespace + } else { + return rootSyncNamespace + } +} + // SetupWithManager sets up the controller with the Manager. func (r *RemoteRootSyncReconciler) SetupWithManager(mgr ctrl.Manager) error { r.channel = make(chan event.GenericEvent, 10) diff --git a/rollouts/controllers/rollout_controller.go b/rollouts/controllers/rollout_controller.go index 6d0bd20557..ef63a09bd8 100644 --- a/rollouts/controllers/rollout_controller.go +++ b/rollouts/controllers/rollout_controller.go @@ -290,14 +290,6 @@ func (r *RolloutReconciler) getPackageDiscoveryClient(rolloutNamespacedName type func (r *RolloutReconciler) reconcileRollout(ctx context.Context, rollout *gitopsv1alpha1.Rollout, strategy *gitopsv1alpha1.ProgressiveRolloutStrategy, packageDiscoveryClient *packagediscovery.PackageDiscovery) error { logger := klog.FromContext(ctx) - if rollout != nil && rollout.Spec.SyncTemplate != nil { - if rollout.Spec.SyncTemplate.Type == gitopsv1alpha1.TemplateTypeRepoSync { - err := fmt.Errorf("reposync is not yet supported") - logger.Error(err, "") - return err - } - } - targetClusters, err := r.store.ListClusters(ctx, &rollout.Spec.Clusters, rollout.Spec.Targets.Selector) discoveredPackages, err := packageDiscoveryClient.GetPackages(ctx, rollout.Spec.Packages) if err != nil { @@ -453,11 +445,11 @@ func (r *RolloutReconciler) computeTargets(ctx context.Context, func pkgNeedsUpdate(rollout *gitopsv1alpha1.Rollout, rrs gitopsv1alpha1.RemoteRootSync, pkg *packagediscovery.DiscoveredPackage) (*gitopsv1alpha1.RemoteRootSync, bool) { // TODO: We need to check other things here besides git.Revision and metadata - if pkg.Revision != rrs.Spec.Template.Spec.Git.Revision || - !reflect.DeepEqual(rollout.Spec.SyncTemplate.RootSync.Metadata, - rrs.Spec.Template.Metadata) { + metadata := getSpecMetadata(rollout) + if pkg.Revision != rrs.Spec.Template.Spec.Git.Revision || !reflect.DeepEqual(metadata, rrs.Spec.Template.Metadata) || rrs.Spec.Type != rollout.Spec.SyncTemplate.Type { rrs.Spec.Template.Spec.Git.Revision = pkg.Revision - rrs.Spec.Template.Metadata = rollout.Spec.SyncTemplate.RootSync.Metadata + rrs.Spec.Template.Metadata = metadata + rrs.Spec.Type = rollout.Spec.SyncTemplate.Type return &rrs, true } return nil, false @@ -541,12 +533,6 @@ func (r *RolloutReconciler) rolloutTargets(ctx context.Context, rollout *gitopsv } } - var metadata *gitopsv1alpha1.Metadata - if rollout != nil && rollout.Spec.SyncTemplate != nil && - rollout.Spec.SyncTemplate.RootSync != nil { - metadata = rollout.Spec.SyncTemplate.RootSync.Metadata - } - for _, target := range targets.ToBeCreated { rootSyncSpec := toRootSyncSpec(target.packageRef) rrs := newRemoteRootSync(rollout, @@ -554,7 +540,7 @@ func (r *RolloutReconciler) rolloutTargets(ctx context.Context, rollout *gitopsv rootSyncSpec, pkgID(target.packageRef), wave.Name, - metadata, + getSpecMetadata(rollout), ) if maxConcurrent > concurrentUpdates { @@ -734,6 +720,9 @@ func newRemoteRootSync(rollout *gitopsv1alpha1.Rollout, t := true clusterName := clusterRef.Name[strings.LastIndex(clusterRef.Name, "/")+1:] + // The RemoteRootSync object is created in the same namespace as the Rollout + // object. The RemoteRootSync will create either a RepoSync in the same namespace, + // or a RootSync in the config-management-system namespace. return &gitopsv1alpha1.RemoteRootSync{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-%s", pkgID, clusterName), @@ -751,7 +740,9 @@ func newRemoteRootSync(rollout *gitopsv1alpha1.Rollout, }, }, }, + Spec: gitopsv1alpha1.RemoteRootSyncSpec{ + Type: rollout.Spec.SyncTemplate.Type, ClusterRef: clusterRef, Template: &gitopsv1alpha1.RootSyncInfo{ Spec: rssSpec, @@ -933,3 +924,22 @@ func filterClusters(allClusters []clusterstore.Cluster, labelSelector *metav1.La return clusters, nil } + +func getSpecMetadata(rollout *gitopsv1alpha1.Rollout) *gitopsv1alpha1.Metadata { + if rollout == nil || rollout.Spec.SyncTemplate == nil { + return nil + } + switch rollout.Spec.SyncTemplate.Type { + case gitopsv1alpha1.TemplateTypeRepoSync: + if rollout.Spec.SyncTemplate.RepoSync == nil { + return nil + } + return rollout.Spec.SyncTemplate.RepoSync.Metadata + case gitopsv1alpha1.TemplateTypeRootSync, "": + if rollout.Spec.SyncTemplate.RootSync == nil { + return nil + } + return rollout.Spec.SyncTemplate.RootSync.Metadata + } + return nil +} diff --git a/rollouts/controllers/status.go b/rollouts/controllers/status.go index f08d5d6149..e42ad6fd89 100644 --- a/rollouts/controllers/status.go +++ b/rollouts/controllers/status.go @@ -18,28 +18,34 @@ import ( "context" "fmt" + gitopsv1alpha1 "github.com/GoogleContainerTools/kpt/rollouts/api/v1alpha1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/dynamic" ) -// checkSyncStatus fetches the RootSync using the provided client and computes the sync status. The rules +// checkSyncStatus fetches the external sync using the provided client and computes the sync status. The rules // for computing status here mirrors the one used in the status command in the nomos cli. -func checkSyncStatus(ctx context.Context, client dynamic.Interface, rrsName string) (string, error) { - // TODO: Change this to use the RootSync type instead of Unstructured. - rs, err := client.Resource(rootSyncGVR).Namespace(rootSyncNamespace).Get(ctx, rrsName, metav1.GetOptions{}) +func checkSyncStatus(ctx context.Context, client dynamic.Interface, rrs *gitopsv1alpha1.RemoteRootSync) (string, error) { + gvr, gvk, err := getGvrAndGvk(rrs.Spec.Type) if err != nil { - return "", fmt.Errorf("failed to get RootSync: %w", err) + return "", err } + rs, err := client.Resource(gvr).Namespace(getExternalSyncNamespace(rrs)).Get(ctx, rrs.Name, metav1.GetOptions{}) + if err != nil { + return "", fmt.Errorf("failed to get %s: %w", gvk.Kind, err) + } + + // TODO: Change this to use the RootSync type instead of Unstructured. generation, _, err := unstructured.NestedInt64(rs.Object, "metadata", "generation") if err != nil { - return "", fmt.Errorf("failed to read generation from RootSync: %w", err) + return "", fmt.Errorf("failed to read generation from %s: %w", gvk.Kind, err) } observedGeneration, _, err := unstructured.NestedInt64(rs.Object, "status", "observedGeneration") if err != nil { - return "", fmt.Errorf("failed to read observedGeneration from RootSync: %w", err) + return "", fmt.Errorf("failed to read observedGeneration from %s: %w", gvk.Kind, err) } if generation != observedGeneration { @@ -48,7 +54,7 @@ func checkSyncStatus(ctx context.Context, client dynamic.Interface, rrsName stri conditions, _, err := unstructured.NestedSlice(rs.Object, "status", "conditions") if err != nil { - return "", fmt.Errorf("failed to extract conditions from RootSync: %w", err) + return "", fmt.Errorf("failed to extract conditions from %s: %w", gvk.Kind, err) } val, found, err := getConditionStatus(conditions, "Stalled")