From 4d2ee4fb7991c35c442fa23dc19dac47444fb44d Mon Sep 17 00:00:00 2001 From: Mario Manno Date: Fri, 20 Oct 2023 18:12:08 +0200 Subject: [PATCH] wip debug --- charts/fleet-crd/templates/crds.yaml | 4 + .../controller/bundledeployment_controller.go | 76 ++++++++++++------- internal/cmd/agent/deployer/deployer.go | 4 +- .../agent/deployer/driftdetect/driftdetect.go | 2 +- .../cmd/agent/deployer/monitor/monitor.go | 3 + .../agent/deployer/monitor/updatestatus.go | 2 + internal/cmd/agent/start.go | 2 +- internal/helmdeployer/deployer.go | 6 +- .../v1alpha1/bundledeployment_types.go | 2 + 9 files changed, 65 insertions(+), 36 deletions(-) diff --git a/charts/fleet-crd/templates/crds.yaml b/charts/fleet-crd/templates/crds.yaml index 75b250c67f..a84847c020 100644 --- a/charts/fleet-crd/templates/crds.yaml +++ b/charts/fleet-crd/templates/crds.yaml @@ -130,6 +130,10 @@ spec: description: DeploymentID is the ID of the currently applied deployment. nullable: true type: string + forceRefresh: + description: ForceRefresh is used to check and if needed redeploy + nullable: true + type: string options: description: Options are the deployment options, that are currently applied. diff --git a/internal/cmd/agent/controller/bundledeployment_controller.go b/internal/cmd/agent/controller/bundledeployment_controller.go index 6691578176..ce0067fe7d 100644 --- a/internal/cmd/agent/controller/bundledeployment_controller.go +++ b/internal/cmd/agent/controller/bundledeployment_controller.go @@ -66,7 +66,7 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req // this call shall not mutate bd status, err := r.Deployer.DeployBundle(ctx, bd) - if err == nil { + if err != nil { // if there is an error, do not use the returned status status = *bd.Status.DeepCopy() } @@ -86,27 +86,27 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, updateErr } - // this call shall not mutate bd + // this call shall not mutate bd - TODO I think it does status, err = r.Monitor.MonitorBundle(ctx, bd) - if err == nil { + if err != nil { // if there is an error, do not use the returned status status = *bd.Status.DeepCopy() } - bd, updateErr = updateCondition(ctx, r.Client, bd, &status, err, condition.Cond(fleetv1.BundleDeploymentConditionMonitored)) - if err != nil { - return ctrl.Result{}, err - } - if updateErr != nil { - logger.V(1).Error(updateErr, "Failed to update bundledeployment status", "step", "monitor", "status", status) - return ctrl.Result{}, updateErr - } - - _, err = r.DriftDetect.Refresh(logger, req.String(), bd) - if err != nil { - logger.V(1).Error(err, "Failed to refresh drift detection", "step", "drift") - return ctrl.Result{}, err - } + // bd, updateErr = updateCondition(ctx, r.Client, bd, &status, err, condition.Cond(fleetv1.BundleDeploymentConditionMonitored)) + // if err != nil { + // return ctrl.Result{}, err + // } + // if updateErr != nil { + // logger.V(1).Error(updateErr, "Failed to update bundledeployment status", "step", "monitor", "status", status) + // return ctrl.Result{}, updateErr + // } + + // _, err = r.DriftDetect.Refresh(logger, req.String(), bd) + // if err != nil { + // logger.V(1).Error(err, "Failed to refresh drift detection", "step", "drift") + // return ctrl.Result{}, err + // } return ctrl.Result{}, nil } @@ -118,10 +118,12 @@ func updateCondition(ctx context.Context, c client.Client, bd *fleetv1.BundleDep } else { cond.SetError(newStatus, "", err) } - if !equality.Semantic.DeepEqual(bd.Status, newStatus) { + if !equality.Semantic.DeepEqual(bd.Status, *newStatus) { // Since status has changed, update the lastUpdatedTime cond.LastUpdated(newStatus, time.Now().UTC().Format(time.RFC3339)) + log.FromContext(ctx).V(1).Info("Updating bundledeployment status", "cond", cond, "from", bd.Status, "to", newStatus) + bd.Status = *newStatus err := c.Status().Update(ctx, bd) // TODO log status update error? @@ -135,16 +137,32 @@ func updateCondition(ctx context.Context, c client.Client, bd *fleetv1.BundleDep func (r *BundleDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&fleetv1.BundleDeployment{}). - WithEventFilter(predicate.Funcs{ - DeleteFunc: func(e event.DeleteEvent) bool { - // This actually deletes the helm releases if a bundledeployment is deleted or orphaned - bd := e.Object.(*fleetv1.BundleDeployment) - key := fmt.Sprintf("%s/%s", e.Object.GetNamespace(), e.Object.GetName()) - r.Cleanup.CleanupReleases(context.Background(), key, bd) - - // return false, so delete won't trigger Reconcile - return false - }, - }). + WithEventFilter( + // we do not trigger for status changes + predicate.And( + predicate.GenerationChangedPredicate{}, + predicate.AnnotationChangedPredicate{}, + predicate.LabelChangedPredicate{}, + predicate.Funcs{ + // // excpect for changes to status.Refresh + // UpdateFunc: func(e event.UpdateEvent) bool { + // n := e.ObjectNew.(*fleetv1.BundleDeployment) + // o := e.ObjectOld.(*fleetv1.BundleDeployment) + // if n == nil || o == nil { + // return false + // } + // return n.Status.ForceRefresh != o.Status.ForceRefresh + // }, + DeleteFunc: func(e event.DeleteEvent) bool { + // This actually deletes the helm releases if a bundledeployment is deleted or orphaned + bd := e.Object.(*fleetv1.BundleDeployment) + key := fmt.Sprintf("%s/%s", e.Object.GetNamespace(), e.Object.GetName()) + r.Cleanup.CleanupReleases(context.Background(), key, bd) + + // return false, so delete won't trigger Reconcile + return false + }, + }, + )). Complete(r) } diff --git a/internal/cmd/agent/deployer/deployer.go b/internal/cmd/agent/deployer/deployer.go index bb4fcb5f97..5ab333ba2e 100644 --- a/internal/cmd/agent/deployer/deployer.go +++ b/internal/cmd/agent/deployer/deployer.go @@ -37,7 +37,7 @@ func NewDeployer(localClient client.Client, upstreamClient client.Reader, lookup } func (d *Deployer) DeployBundle(ctx context.Context, bd *fleet.BundleDeployment) (fleet.BundleDeploymentStatus, error) { - logger := log.FromContext(ctx).WithName("DeployBundle").WithValues("deploymentID", bd.Spec.DeploymentID, "appliedDeploymentID", bd.Status.AppliedDeploymentID) + logger := log.FromContext(ctx).WithName("DeployBundle").WithValues("deployment ID", bd.Spec.DeploymentID, "applied deployment ID", bd.Status.AppliedDeploymentID) status := *bd.Status.DeepCopy() if bd.Spec.Paused { @@ -51,7 +51,7 @@ func (d *Deployer) DeployBundle(ctx context.Context, bd *fleet.BundleDeployment) return status, err } - logger.Info("Deploying bundle") + logger.Info("Checking if bundle needs to be deployed") release, err := d.helmdeploy(ctx, bd) if err != nil { // When an error from DeployBundle is returned it causes DeployBundle diff --git a/internal/cmd/agent/deployer/driftdetect/driftdetect.go b/internal/cmd/agent/deployer/driftdetect/driftdetect.go index 3f4f8e1154..dcfd51e4af 100644 --- a/internal/cmd/agent/deployer/driftdetect/driftdetect.go +++ b/internal/cmd/agent/deployer/driftdetect/driftdetect.go @@ -35,7 +35,7 @@ func New( return &DriftDetect{ trigger: trigger, deployer: deployer, - apply: apply, + apply: apply.WithDynamicLookup(), defaultNamespace: defaultNamespace, labelPrefix: labelPrefix, labelSuffix: labelSuffix, diff --git a/internal/cmd/agent/deployer/monitor/monitor.go b/internal/cmd/agent/deployer/monitor/monitor.go index 7aea6fbd39..85d0508aa9 100644 --- a/internal/cmd/agent/deployer/monitor/monitor.go +++ b/internal/cmd/agent/deployer/monitor/monitor.go @@ -75,6 +75,9 @@ func (ma *Monitor) MonitorBundle(ctx context.Context, bd *fleet.BundleDeployment condition.Cond(fleet.BundleDeploymentConditionReady).SetError(&status, "", readyError) if len(status.ModifiedStatus) > 0 { // TODO(manno): this should be a return value or a status update ...but how to delay? + // TODO + // TODO + // TODO //h.bdController.EnqueueAfter(bd.Namespace, bd.Name, durations.MonitorBundleDelay) if shouldRedeploy(bd) { logger.Info("Redeploying") diff --git a/internal/cmd/agent/deployer/monitor/updatestatus.go b/internal/cmd/agent/deployer/monitor/updatestatus.go index bc15292047..71d74bd6c7 100644 --- a/internal/cmd/agent/deployer/monitor/updatestatus.go +++ b/internal/cmd/agent/deployer/monitor/updatestatus.go @@ -64,6 +64,8 @@ func (m *Monitor) updateBundleDeploymentStatus(ctx context.Context, bd *fleet.Bu } else if bd.Spec.CorrectDrift.Enabled { err = m.Deployer.RemoveExternalChanges(ctx, bd) if err != nil { + // TODO move up to caller + // TODO we just mutated bd // Update BundleDeployment status as wrangler doesn't update the status if error is not nil. errStatus := m.UpstreamClient.Status().Update(ctx, bd) if errStatus != nil { diff --git a/internal/cmd/agent/start.go b/internal/cmd/agent/start.go index 2b8e75eec8..c109af86d8 100644 --- a/internal/cmd/agent/start.go +++ b/internal/cmd/agent/start.go @@ -145,7 +145,7 @@ func start(ctx context.Context, kubeConfig, systemNamespace, agentScope string) getter := cli.New().RESTClientGetter() // uses local kubeconfig helmDeployer, _ := helmdeployer.NewHelm(ctx, localClient, systemNamespace, defaultNamespace, defaultNamespace, agentScope, getter) - // Build the deployer handler's that the bundledeployment reconciler will use + // Build the deployer that the bundledeployment reconciler will use deployer := deployer.NewDeployer(localClient, mgr.GetAPIReader(), manifest.NewLookup(), helmDeployer) apply, err := apply.NewForConfig(localconfig) diff --git a/internal/helmdeployer/deployer.go b/internal/helmdeployer/deployer.go index 6ca9adfde8..345561f114 100644 --- a/internal/helmdeployer/deployer.go +++ b/internal/helmdeployer/deployer.go @@ -352,7 +352,7 @@ func (h *Helm) getCfg(ctx context.Context, namespace, serviceAccountName string) // install runs helm install or upgrade and supports dry running the action. Will run helm rollback in case of a failed upgrade. func (h *Helm) install(ctx context.Context, bundleID string, manifest *manifest.Manifest, chart *chart.Chart, options fleet.BundleDeploymentOptions, dryRun bool) (*release.Release, error) { - logger := log.FromContext(ctx).WithName("HelmDeployer").WithName("install").WithValues("commit", manifest.Commit, "dryRun", dryRun) + logger := log.FromContext(ctx).WithName("HelmDeployer").WithName("install").WithValues("commit", manifest.Commit, "dry run", dryRun) timeout, defaultNamespace, releaseName := h.getOpts(bundleID, options) values, err := h.getValues(ctx, options, defaultNamespace) @@ -680,7 +680,7 @@ func (h *Helm) Delete(ctx context.Context, bundleID, releaseName string) error { } func (h *Helm) deleteByRelease(ctx context.Context, bundleID, releaseName string, keepResources bool) error { - logger := log.FromContext(ctx).WithName("deleteByRelease").WithValues("releaseName", releaseName, "keepResources", keepResources) + logger := log.FromContext(ctx).WithName("deleteByRelease").WithValues("release name", releaseName, "keep resources", keepResources) releaseNamespace, releaseName := kv.Split(releaseName, "/") rels, err := h.globalCfg.Releases.List(func(r *release.Release) bool { return r.Namespace == releaseNamespace && @@ -726,7 +726,7 @@ func (h *Helm) deleteByRelease(ctx context.Context, bundleID, releaseName string } func (h *Helm) delete(ctx context.Context, bundleID string, options fleet.BundleDeploymentOptions, dryRun bool) error { - logger := log.FromContext(ctx).WithName("HelmDeployer").WithName("delete").WithValues("dryRun", dryRun) + logger := log.FromContext(ctx).WithName("HelmDeployer").WithName("delete").WithValues("dry run", dryRun) timeout, _, releaseName := h.getOpts(bundleID, options) r, err := h.globalCfg.Releases.Last(releaseName) diff --git a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go index 924efff186..4a31841cc9 100644 --- a/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go +++ b/pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go @@ -243,6 +243,8 @@ type BundleDeploymentSpec struct { DependsOn []BundleRef `json:"dependsOn,omitempty"` // CorrectDrift specifies how drift correction should work. CorrectDrift CorrectDrift `json:"correctDrift,omitempty"` + // ForceRefresh is used to check and if needed redeploy + ForceRefresh metav1.Time `json:"forceRefresh,omitempty"` } // BundleDeploymentResource contains the metadata of a deployed resource.