Support to have an eventual phase that can be run at the end of every action

[viveksinghggits]

@glothriel, was kind enough give this feedback to have an eventual phase for every action to make sure we are able to cleanup the resources if they were created in previous phases.
I am creating this thread to continue the discussion and if you have any other ideas around it and then we can add this to the roadmap.

• Eventual phase should be called after all the phases of an action are called
• It should be called irrespective of the status of previous phases

[viveksinghggits]

Hi @pavannd1 @ihcsim
Instead of having a field AlwaysRun in the type BlueprintPhase, I was thinking of having a field called EventualPhase in the BlueprintAction struct. Something like this

type BlueprintAction struct {
	Name               string              `json:"name"`
	Kind               string              `json:"kind"`
	ConfigMapNames     []string            `json:"configMapNames,omitempty"`
	SecretNames        []string            `json:"secretNames,omitempty"`
	InputArtifactNames []string            `json:"inputArtifactNames,omitempty"`
	OutputArtifacts    map[string]Artifact `json:"outputArtifacts,omitempty"`
	Phases             []BlueprintPhase    `json:"phases,omitempty"`
        EventualPhase BlueprintPhase    `json:"eventualPhase,omitempty"`
}

Having the eventual phse this way won't require any additional logic to make sure that eventual phase is actually running at the very end. Otherwise if we include a field in BlueprintPhase type we will have to make sure that we are running that phase after every other phase for that action is completed (success/failure).
what do you think about this.

[viveksinghggits]

also, the other thing that I observed is there is no need to introduce preHook. The thing that we are trying to achieve using preHook can easily be achieved using phases of a function.

[ihcsim]

I am hesitant to call them hooks because hooks are usually event-driven. In our case, we want a way to order the phases, ensure that they are always run, and their errors are handled differently.

I don't see why we can't use []BlueprintPhase. Are you aware of any issues?

Do we have any mechanism to mark this feature as experimental (at least in the docs), indicating that we might rename, deprecate, redesign it in subsequent releases?

[viveksinghggits]

Hi @ihcsim

In our case, we want a way to order the phases, ensure that they are always run, and their errors are handled differently.

this can be achieved today by just having phases in actions. And that is the reason I don't want to have preHook because, in that case we are basically having two ways to do the exact same thing which I want to avoid.

I don't think we have any issues with having []BlueprintPhase as type. We can do that.
We have not marked features as experimental before but if there is a need we can do that, I think.

[pavannd1]

I think we should have both pre and post for consistency and maybe add enough documentation to educate authors on the use of pre vs just having a phase in phases.

[viveksinghggits]

Continuing in another comment this is getting too lengthy.

[viveksinghggits]

I think we should have both pre and post for consistency and maybe add enough documentation to educate authors on the use of pre vs just having a phase in phases.

I somehow still think that we don't need pre hook because of below reasons

• We will have to put extra efforts in docs to specify how to use hooks
• Since same thing can be achieved by two ways, we are making things confusing

I also don't think we have specified or used pre hooks like this before (in other blueprints) and changing that is going to be problem with consistency. What do you think @pavannd1 @ihcsim .

[mandreasik]

Hi guys :)

I was asked by @glothriel to prepare short summary based on our use cases of blueprints. So let's get started

use case based on our "restore" action:

note:
custom operator takes care of creating custom database, users etc on db cluster

1. scale custom operator down (we don't want any unpredictable actions to be taken on database when performing restore)
2. restore mysql database (whole server or particular database)
3. scale custom operator up

If restore of database will fail due to any reason our custom operator is not running which means that part of our application is not working correctly, and we need to manually restore scale of our custom operator

use case based on our "backup" action:
Right now kanister does not support backup rotation, post actions like "cleanup" or cleaning up failed actionsets (eg. preserving only last two recent or so) so we had to code it by ourselves. We did it by adding second phase to "backup" action.

Having this in mind all our backup actions in blueprint look like:

1. prepare data (ew. mount some volumes) and perform backup to remote storage using restic
2. "rotate backup"
   • run restic cleanup - in case previous actionset failed and we have data block not connected to particular restic snapshot
   • run restic backup rotation
   • cleanup failed actionsets preserving only last two recently failed

So if backup phase will fail, we are left with: failed actionsets (that are potentially piling up) and potential unused trash data inside restic repository.

Of course next successful backup would fix this issue but what if there is a bug and backup is failing few times? this could lead to more trash data inside restic repo.

Having special "finally" phase that would fire, regardless backup failed or not would fix above case

[viveksinghggits]

Hi @mandreasik,
thanks for the comment, we have started implementing this and this is how the blueprint should look like

apiVersion: cr.kanister.io/v1alpha1
kind: Blueprint
metadata:
  name: mysql-blueprint
actions:
  backup:
    phases:
    - func: KubeTask
      name: phaseOne
      args:
        image: ghcr.io/kanisterio/mysql-sidecar:0.74.0
        namespace: "{{ .StatefulSet.Namespace }}"
        command:
    - func: KubeTask
      name: phaseTwo
      args:
        image: ghcr.io/kanisterio/mysql-sidecar:0.74.0
        namespace: "{{ .StatefulSet.Namespace }}"
        command:
    - func: KubeTask
      name: phaseThree
      args:
        image: ghcr.io/kanisterio/mysql-sidecar:0.74.0
        namespace: "{{ .StatefulSet.Namespace }}"
        command:
    deferPhase:
      func: KubeTask
      name: finalPhase
      args:
        image: ghcr.io/kanisterio/mysql-sidecar:0.74.0
        namespace: "{{ .StatefulSet.Namespace }}"
        command:

as you can assume by looking at this blueprint, deferPhase going to run eventually irrespective of the status of the main phases that are specified in the blueprint. Please let us know what do you think.
Sorry for the delay in response.

[mandreasik]

@viveksinghggits it looks nice.

It might be silly question but does "deferPhase" can also have multiple functions in it? Is "deferPhase" mandatory or optional?

I also want to ask about status of "actionset" with "deferPhase" (Im not sure whether it is an issue or not but it's worth asking)

Let's imagine following scenarios:

1. "backup phase" succeeded and "deferPahse" succeeded - actionset status is completed
2. "backup phase" succeeded and "deferPhase" failed - actionset is failed?
3. "backup phase" failed and "deferPhase" succeeded - actionset is failed?
4. "backup phase" failed and "deferPhase" failed - actionset is failed?

will there be any easy way to know which scenario occurred besides looking into operator logs? Do you eg. plan to extend actionset status with "phase status" or sth?

[viveksinghggits]

Hi @mandreasik,
Those are good questions.

can also have multiple functions in it?

No, we decided to just have one phase/function as deferPhase because the whole purpose to this deferPhase is to do just some cleanup things. If there are something that are not that trivial and involve more phases I think having them as separate action would be a better idea.

Is "deferPhase" mandatory or optional?

It is optional.

If we talk about the status of actionset

1. Yes ✔️ Below is how the events of actionset going to look like

Events:
  Type    Reason              Age   From                 Message
  ----    ------              ----  ----                 -------
  Normal  Started Action      55s   Kanister Controller  Executing action backup
  Normal  Started Phase       55s   Kanister Controller  Executing phase dumpToObjectStore
  Normal  Ended Phase         50s   Kanister Controller  Completed phase dumpToObjectStore
  Normal  Started Phase       50s   Kanister Controller  Executing phase testSecondPhase
  Normal  Ended Phase         48s   Kanister Controller  Completed phase testSecondPhase
  Normal  Started deferPhase  48s   Kanister Controller  Executing deferPhase deferPhase
  Normal  Ended deferPhase    46s   Kanister Controller  Completed deferPhase deferPhase
  Normal  Update Complete     46s   Kanister Controller  Updated ActionSet 'backup-glgp4' Status->complete

2. Yes ✔️ This is how the events of actionset are going to look like

Events:
  Type     Reason                          Age   From                 Message
  ----     ------                          ----  ----                 -------
  Normal   Started Action                  2m3s  Kanister Controller  Executing action backup
  Normal   Started Phase                   2m3s  Kanister Controller  Executing phase dumpToObjectStore
  Normal   Ended Phase                     118s  Kanister Controller  Completed phase dumpToObjectStore
  Normal   Started Phase                   118s  Kanister Controller  Executing phase testSecondPhase
  Normal   Ended Phase                     116s  Kanister Controller  Completed phase testSecondPhase
  Normal   Started deferPhase              116s  Kanister Controller  Executing deferPhase deferPhase
  Warning  ActionSetFailed Action: backup  115s  Kanister Controller  Failed to execute defer phase: errormessage

3. Yes ✔️ Below is how events are going to look like

Events:
  Type     Reason                          Age   From                 Message
  ----     ------                          ----  ----                 -------
  Normal   Started Action                  79s   Kanister Controller  Executing action backup
  Normal   Started Phase                   79s   Kanister Controller  Executing phase dumpToObjectStore
  Normal   Ended Phase                     74s   Kanister Controller  Completed phase dumpToObjectStore
  Normal   Started Phase                   74s   Kanister Controller  Executing phase testSecondPhase
  Warning  ActionSetFailed Action: backup  74s   Kanister Controller  Failed to execute phase: errormessage
  Normal   Started deferPhase              74s   Kanister Controller  Executing deferPhase deferPhase
  Normal   Ended deferPhase                72s   Kanister Controller  Completed deferPhase deferPhase

4. Yes ✔️ Below is how the events in actionset look like

Events:
  Type     Reason                          Age   From                 Message
  ----     ------                          ----  ----                 -------
  Normal   Started Action                  118s  Kanister Controller  Executing action backup
  Normal   Started Phase                   118s  Kanister Controller  Executing phase dumpToObjectStore
  Normal   Ended Phase                     114s  Kanister Controller  Completed phase dumpToObjectStore
  Normal   Started Phase                   114s  Kanister Controller  Executing phase testSecondPhase
  Warning  ActionSetFailed Action: backup  112s  Kanister Controller  Failed to execute phase: errormessage
  Normal   Started deferPhase              112s  Kanister Controller  Executing deferPhase deferPhase
  Warning  ActionSetFailed Action: backup  110s  Kanister Controller  Failed to execute defer phase: errormessage

Please feel free to ask/suggest if you have anything.

[ihcsim]

This is implemented in #1297.