Skip to content

Commit

Permalink
chore: refactoring so that addon controller flow won't inquire from w…
Browse files Browse the repository at this point in the history 
…orkflow obj (#251)

* refactoring so that addon controller flow shouldn't update status from workflow obj
* simplifying status updates from addon controller
* fixing Finalize flow to not require status from wf obj
* runWorkflow sets statuses instead of returning them
* renamed SetPrereqStatus to SetPrereqAndInstallStatuses and removed setting status reasons outside of addon_types funcs
* covering setting install status in cause remove finalizer fails to update
---------

Signed-off-by: Chun-Che Peng <[email protected]>
  • Loading branch information
@ccpeng
ccpeng authored Jun 30, 2023
1 parent 20796d4 commit 81b57f9
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 99 deletions.
25 changes: 23 additions & 2 deletions api/addon/v1alpha1/addon_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,27 @@ func (a *Addon) CalculateChecksum() string {
return fmt.Sprintf("%x", adler32.Checksum([]byte(fmt.Sprintf("%+v", a.Spec))))
}

func (a *Addon) SetStatusByLifecyleStep(step LifecycleStep, phase ApplicationAssemblyPhase, reasons ...string) error {
var err error

switch step {
case Prereqs:
err = a.SetPrereqAndInstallStatuses(phase, reasons...)
case Install:
a.SetInstallStatus(phase, reasons...)
case Delete:
if phase == Succeeded {
a.SetInstallStatus(DeleteSucceeded, reasons...)
} else if phase == Failed {
a.SetInstallStatus(DeleteFailed, reasons...)
} else {
a.SetInstallStatus(phase, reasons...)
}
}

return err
}

// GetInstallStatus returns the install phase for addon
func (a *Addon) GetInstallStatus() ApplicationAssemblyPhase {
return a.Status.Lifecycle.Installed
Expand All @@ -403,8 +424,8 @@ func (a *Addon) GetPrereqStatus() ApplicationAssemblyPhase {
return a.Status.Lifecycle.Prereqs
}

// SetPrereqStatus sets the prereq phase for addon
func (a *Addon) SetPrereqStatus(phase ApplicationAssemblyPhase, reasons ...string) error {
// SetPrereqAndInstallStatuses sets the prereq and install phases for addon
func (a *Addon) SetPrereqAndInstallStatuses(phase ApplicationAssemblyPhase, reasons ...string) error {

switch phase {
case Pending:
Expand Down
61 changes: 39 additions & 22 deletions controllers/addon_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ func (r *AddonReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
return reconcile.Result{}, ignoreNotFound(err)
}

return r.execAddon(ctx, req, log, instance)
return r.execAddon(ctx, log, instance)
}

func (r *AddonReconciler) execAddon(ctx context.Context, req reconcile.Request, log logr.Logger, instance *addonmgrv1alpha1.Addon) (reconcile.Result, error) {
func (r *AddonReconciler) execAddon(ctx context.Context, log logr.Logger, instance *addonmgrv1alpha1.Addon) (reconcile.Result, error) {
defer func() {
if err := recover(); err != nil {
log.Info(fmt.Sprintf("Error: Panic occurred during execAdd %s/%s due to %v", instance.Namespace, instance.Name, err))
Expand All @@ -154,7 +154,6 @@ func (r *AddonReconciler) execAddon(ctx context.Context, req reconcile.Request,
reason := fmt.Sprintf("Addon %s/%s could not be finalized. %v", instance.Namespace, instance.Name, err)
r.recorder.Event(instance, "Warning", "Failed", reason)
log.Error(err, "Failed to finalize addon.")
instance.SetInstallStatus(addonmgrv1alpha1.DeleteFailed, reason)

return reconcile.Result{}, err
}
Expand Down Expand Up @@ -245,13 +244,13 @@ func (r *AddonReconciler) processAddon(ctx context.Context, log logr.Logger, ins

if changedStatus {
// Set ttl starttime if checksum has changed
instance.Status.StartTime = common.GetCurretTimestamp()
instance.Status.StartTime = common.GetCurrentTimestamp()

// Clear out status and reason
instance.ClearStatus()

log.Info("Checksum changed, addon will be installed...")
instance.SetInstallStatus(addonmgrv1alpha1.Pending)
instance.SetPrereqAndInstallStatuses(addonmgrv1alpha1.Pending)
log.Info("Requeue to set pending status")
return reconcile.Result{Requeue: true}, nil
}
Expand Down Expand Up @@ -314,7 +313,7 @@ func (r *AddonReconciler) processAddon(ctx context.Context, log logr.Logger, ins
// Execute PreReq and Install workflow, if spec body has changed.
// In the case when validation failed and continued here we should execute.
// Also, if workflow is in Pending state, execute it to update status to terminal state.
if instance.Status.Lifecycle.Installed.Completed() == false {
if !instance.Status.Lifecycle.Installed.Completed() {
// Check if addon installation expired.
if common.IsExpired(instance.Status.StartTime, addonapiv1.TTL.Milliseconds()) {
reason := fmt.Sprintf("Addon %s/%s ttl expired, starttime exceeded %s", instance.Namespace, instance.Name, addonapiv1.TTL.String())
Expand Down Expand Up @@ -359,31 +358,47 @@ func ignoreNotFound(err error) error {
return err
}

func (r *AddonReconciler) runWorkflow(ctx context.Context, lifecycleStep addonmgrv1alpha1.LifecycleStep, addon *addonmgrv1alpha1.Addon, wfl workflows.AddonLifecycle) (addonmgrv1alpha1.ApplicationAssemblyPhase, error) {
// runWorkflow attempts creation of wf if corresponding lifecycleStep is not already completed
func (r *AddonReconciler) runWorkflow(ctx context.Context, lifecycleStep addonmgrv1alpha1.LifecycleStep, addon *addonmgrv1alpha1.Addon, wfl workflows.AddonLifecycle) error {
log := r.Log.WithValues("addon", fmt.Sprintf("%s/%s", addon.Namespace, addon.Name))

if lifecycleStep == addonmgrv1alpha1.Prereqs && addon.GetPrereqStatus().Completed() {
log.Info("Lifecycle completed, skipping workflow execution", "lifecycleStep", lifecycleStep)
return nil
} else if lifecycleStep == addonmgrv1alpha1.Install && addon.GetInstallStatus().Completed() {
log.Info("Lifecycle completed, skipping workflow execution", "lifecycleStep", lifecycleStep)
return nil
} else if lifecycleStep == addonmgrv1alpha1.Delete && (addon.Status.Lifecycle.Installed == addonmgrv1alpha1.DeleteFailed || addon.Status.Lifecycle.Installed == addonmgrv1alpha1.DeleteSucceeded) {
log.Info("Lifecycle completed, skipping workflow execution", "lifecycleStep", lifecycleStep)
return nil
}

wt, err := addon.GetWorkflowType(lifecycleStep)
if err != nil {
log.Error(err, "lifecycleStep is not a field in LifecycleWorkflowSpec", "lifecycleStep", lifecycleStep)
return addonmgrv1alpha1.Failed, err
addon.SetStatusByLifecyleStep(lifecycleStep, addonmgrv1alpha1.Failed)
return err
}

if wt.Template == "" {
log.Info("Workflow template is empty, skipping workflow execution", "lifecycleStep", lifecycleStep)
// No workflow was provided, so mark as succeeded
return addonmgrv1alpha1.Succeeded, nil
addon.SetStatusByLifecyleStep(lifecycleStep, addonmgrv1alpha1.Succeeded)
return nil
}

wfIdentifierName := addon.GetFormattedWorkflowName(lifecycleStep)
if wfIdentifierName == "" {
return addonmgrv1alpha1.Failed, fmt.Errorf("could not generate workflow template name")
addon.SetStatusByLifecyleStep(lifecycleStep, addonmgrv1alpha1.Failed)
return fmt.Errorf("could not generate workflow template name")
}
phase, err := wfl.Install(ctx, workflows.NewWorkflowProxy(wfIdentifierName, wt, lifecycleStep))
err = wfl.Install(ctx, workflows.NewWorkflowProxy(wfIdentifierName, wt, lifecycleStep))
if err != nil {
return phase, err
addon.SetStatusByLifecyleStep(lifecycleStep, addonmgrv1alpha1.Failed)
return err
}
r.recorder.Event(addon, "Normal", "Submitted", fmt.Sprintf("Submitted %s workflow %s/%s as phase %s.", strings.Title(string(lifecycleStep)), addon.Namespace, wfIdentifierName, phase))
return phase, nil
r.recorder.Event(addon, "Normal", "Submitted", fmt.Sprintf("Submitted %s workflow %s/%s.", strings.Title(string(lifecycleStep)), addon.Namespace, wfIdentifierName))
return nil
}

func (r *AddonReconciler) validateSecrets(ctx context.Context, addon *addonmgrv1alpha1.Addon) error {
Expand All @@ -406,19 +421,19 @@ func (r *AddonReconciler) validateSecrets(ctx context.Context, addon *addonmgrv1
return nil
}

// executePrereqAndInstall kicks off Prereq/Install workflows and sets ONLY Failed/Succeeded as statuses
func (r *AddonReconciler) executePrereqAndInstall(ctx context.Context, log logr.Logger, instance *addonmgrv1alpha1.Addon, wfl workflows.AddonLifecycle) error {
// Execute PreReq workflow
prereqsPhase, err := r.runWorkflow(ctx, addonmgrv1alpha1.Prereqs, instance, wfl)
err := r.runWorkflow(ctx, addonmgrv1alpha1.Prereqs, instance, wfl)
if err != nil {
reason := fmt.Sprintf("Addon %s/%s prereqs failed. %v", instance.Namespace, instance.Name, err)
r.recorder.Event(instance, "Warning", "Failed", reason)
log.Error(err, "Addon prereqs workflow failed.")
_ = instance.SetPrereqStatus(addonmgrv1alpha1.Failed, reason)

return err
}
_ = instance.SetPrereqStatus(prereqsPhase)

// Validate and Install workflow
if instance.Status.Lifecycle.Prereqs.Succeeded() {
if err := r.validateSecrets(ctx, instance); err != nil {
reason := fmt.Sprintf("Addon %s/%s could not validate secrets. %v", instance.Namespace, instance.Name, err)
Expand All @@ -429,16 +444,16 @@ func (r *AddonReconciler) executePrereqAndInstall(ctx context.Context, log logr.
return err
}

phase, err := r.runWorkflow(ctx, addonmgrv1alpha1.Install, instance, wfl)
err := r.runWorkflow(ctx, addonmgrv1alpha1.Install, instance, wfl)
if err != nil {
reason := fmt.Sprintf("Addon %s/%s could not be installed due to error. %v", instance.Namespace, instance.Name, err)
r.recorder.Event(instance, "Warning", "Failed", reason)
log.Error(err, "Addon install workflow failed.")
instance.SetInstallStatus(addonmgrv1alpha1.Failed, reason)

return err
}
instance.SetInstallStatus(phase)
} else if instance.Status.Lifecycle.Prereqs.Failed() {
instance.SetInstallStatus(addonmgrv1alpha1.Failed) // reason already set in SetPrereqAndInstallStatuses(Failed)
}

return nil
Expand Down Expand Up @@ -530,12 +545,12 @@ func (r *AddonReconciler) Finalize(ctx context.Context, addon *addonmgrv1alpha1.
removeFinalizer = false

// Run delete workflow
phase, err := r.runWorkflow(ctx, addonmgrv1alpha1.Delete, addon, wfl)
err := r.runWorkflow(ctx, addonmgrv1alpha1.Delete, addon, wfl)
if err != nil {
return err
}

if phase.Succeeded() {
if addon.GetInstallStatus().Succeeded() {
// Wait for workflow to succeed.
removeFinalizer = true
}
Expand All @@ -548,6 +563,8 @@ func (r *AddonReconciler) Finalize(ctx context.Context, addon *addonmgrv1alpha1.
if removeFinalizer {
controllerutil.RemoveFinalizer(addon, finalizerName)
if err := r.Update(ctx, addon); err != nil {
reason := fmt.Sprintf("Addon %s/%s could not be deleted, %v", addon.Namespace, addon.Name, err)
addon.SetInstallStatus(addonmgrv1alpha1.DeleteFailed, reason)
return err
}
}
Expand Down
11 changes: 4 additions & 7 deletions pkg/addon/addon_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ func (c *AddonUpdater) UpdateStatus(ctx context.Context, log logr.Logger, addon
return err
}

log.Info("successfully updated addon statuses", "prereqs_status", addon.GetPrereqStatus(), "installed_status", addon.GetInstallStatus())

// Always update the version cache
c.addAddonToCache(log, addon)

Expand Down Expand Up @@ -146,13 +148,8 @@ func (c *AddonUpdater) UpdateAddonStatusLifecycleFromWorkflow(ctx context.Contex
reason = wf.Status.Message
}

if lifecycle == addonmgrv1alpha1.Prereqs {
err := existingAddon.SetPrereqStatus(phase, reason)
if err != nil {
return fmt.Errorf("failed to update prereqs status. %w", err)
}
} else {
existingAddon.SetInstallStatus(phase, reason)
if err := existingAddon.SetStatusByLifecyleStep(lifecycle, phase, reason); err != nil {
return fmt.Errorf("failed to update prereqs status. %w", err)
}

return c.UpdateStatus(ctx, c.log, existingAddon)
Expand Down
14 changes: 0 additions & 14 deletions pkg/addon/addon_validator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,6 @@ func NewAddonValidator(addon *addonmgrv1alpha1.Addon, cache VersionCacheClient,
}
}

func IsDuplicate(addon *addonmgrv1alpha1.Addon, cache VersionCacheClient) bool {
var version = &Version{
Name: addon.GetName(),
Namespace: addon.GetNamespace(),
PackageSpec: addon.GetPackageSpec(),
PkgPhase: addon.Status.Lifecycle.Installed,
}
if v := cache.GetVersion(version.PkgName, version.PkgVersion); v != nil && v.Name != version.Name {
return true
}

return false
}

func (av *addonValidator) Validate() (bool, error) {
var version = &Version{
Name: av.addon.GetName(),
Expand Down
6 changes: 3 additions & 3 deletions pkg/common/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ func RemoveString(slice []string, s string) (result []string) {
return
}

// GetCurretTimestamp -- get current timestamp in millisecond
func GetCurretTimestamp() int64 {
// GetCurrentTimestamp -- get current timestamp in millisecond
func GetCurrentTimestamp() int64 {
return time.Now().UnixMilli()
}

// IsExpired --- check if reached ttl time
func IsExpired(startTime int64, ttlTime int64) bool {
if GetCurretTimestamp()-startTime >= ttlTime {
if GetCurrentTimestamp()-startTime >= ttlTime {
return true
}
return false
Expand Down
4 changes: 2 additions & 2 deletions pkg/common/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ import (

func TestGetCurretTimestamp(t *testing.T) {
g := gomega.NewGomegaWithT(t)
timestamp := GetCurretTimestamp()
timestamp := GetCurrentTimestamp()
expectedTime := time.Unix(0, timestamp*int64(time.Millisecond))
g.Expect(expectedTime).Should(gomega.BeTemporally("~", time.Now(), time.Second))
}

func TestIsExpired(t *testing.T) {
g := gomega.NewGomegaWithT(t)
startTime := GetCurretTimestamp() - 1000
startTime := GetCurrentTimestamp() - 1000
ttlTime := int64(2000)
g.Expect(IsExpired(startTime, ttlTime)).To(gomega.BeFalse())
ttlTime = 0
Expand Down
Loading

0 comments on commit 81b57f9

Please sign in to comment.