Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(docs): add ADR for Plugin lifecycle #14

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

IvoGoman
Copy link
Contributor

@IvoGoman IvoGoman commented Jul 25, 2024

TL;DR;

This ADR proposes to introduce PluginDefinitionRevisions to enable version pinning on PluginPreset and Plugin resources. PluginPresets will support semantic version constraints, while Plugins, due to technical limitations, will only support concrete versions.
This will enable to stagger a rollout of a new PluginDefinition version by using a concrete version tag on the PluginPreset referring to this PluginDefinition. There is still the option to perform automatic upgrades as before.
Greenhouse will provide the means to use any given CD/GitOps/etc. way to deploy these manifests, but it will not provide an out of the box way to deploy the Greenhouse manifests of an Organization into it's namespace.
Garbage-collection of these revisions is proposed to delete the oldest unused revision/version over a count of maxRevision. This way gradually over time older revisions can be removed as soon they are no longer used.

@IvoGoman IvoGoman requested a review from a team as a code owner July 25, 2024 15:17
Copy link

github-actions bot commented Sep 9, 2024

This PR is stale because it has been open for 45 days with no activity.

@github-actions github-actions bot added the stale label Sep 9, 2024
Copy link

This PR was closed because it has been inactive for 14 days since being marked as stale.

@github-actions github-actions bot closed this Sep 23, 2024
@kengou kengou reopened this Sep 23, 2024
@kengou kengou added backlog and removed stale labels Sep 23, 2024
| [decision driver c] | -- | Bad, because [argument c] |
| [decision driver d] | o | Neutral, because [argument d] |


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### [`UpdateDelay` field on `PluginPreset.Spec`]
Description: Like PluginPreset Orchestrator idea with no explicit version pinning
In this approach the PluginPreset would be extended to allow an optional `UpdateDelay` value (within a defined range?). This value would default to `0`. (need to decide on Syntax... CronJob-like?)
This would provide the possibility to stagger rollouts without the need for version pinning.
The `PluginPreset` Controller would need to maintain the timestamp of the last Update of the `PluginDefinition`. If `now - UpdateTimestamp > UpdateDelay` the controller will perform the update. It will set an `UpdateDelayedCondition` otherwise to inform the User of a pending Update. We might extend this Condition to contain the `diff` of the changes.

@IvoGoman @kengou (please mention any other decision drivers)

This might be a first step to a simple approach to solve the pressing issues. We might want to keep (opinionated) simplicity on this one. Forcing Updates, but allowing delays and therefore staggering. A little bit like the Gardener Way?

We might even decide to set the PluginPreset into an Error state to make sure a delayed update is perceived. Or we introduce a more generic WarningReceivedCondition on all our resources, so the UI is able to display these.

We do have to think what happens if an new Update happens within the UpdateDelay period. If the Controller just updates the timestamp the whole process starts again, but the running Plugin will be 2 versions behind. It would make sure we only update the PluginPreset if we have the Update running without interruption for the complete UpdateDelay period.

@IvoGoman IvoGoman changed the title feat: init plugin lifecycle adr feat(docs): add ADR for Plugin lifecycle Feb 26, 2025
Comment on lines 360 to 361
To address these issues a new PluginDefinitionRevision CRD is proposed. The PluginDefinitionRevision should similar to a revision of a Helm release provide a snapshot of the PluginDefinition at a certain version. The PluginDefinitionRevision is immutable and stores the `version`, `pluginOptions` and `helmChart` of a PluginDefinition. There may be multiple revisions for a single PluginDefinition.
Instead of referencing a PluginDefinition, a Plugin and PluginPreset should reference a PluginDefinitionRevision. This will allow to stage rollouts of new PluginDefinitions, as well as provide a way to rollback to a previous version of a PluginDefinition. Also updates to a PluginDefinition will not affect an in progress rollout.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach a lot. The CNCF project crossplane also has the same behaviour and it works very well. Here is the link to their documentation.

Note

When Composition Revisions are enabled three things happen:
Crossplane creates a CompositionRevision for each Composition update.
Composite Resources gain a spec.compositionRevisionRef field that specifies which CompositionRevision they use.
Composite Resources gain a spec.compositionUpdatePolicy field that specifies how they should be updated to new Composition Revisions.

  • Automatic: Automatically use the latest CompositionRevision. (Default)
  • Manual: Require manual intervention to change CompositionRevision.

Each time you edit a Composition Crossplane will automatically create a CompositionRevision that represents that ‘revision’ of the Composition - that unique state. Each revision is allocated an increasing revision number. This gives CompositionRevision consumers an idea about which revision is ’newest’.

We can do the same with PluginDefinition and PluginDefinitionRevision :)

Comment on lines 404 to 421
The PluginPreset already manages some aspects of the Plugin lifecycle. Additionally, it could be extended to support semantic version constraints as part of this change. This would allow for semi-automatic updates if desired. In such a scenario, the PluginPreset could have a semantic version constraint on a certain minor version but allow any patch versions. (e.g. `^v1.1.x`) In this scenario any new PluginDefinitionRevision that brings a higher patch level would automatically be deployed.
This version constraint will not be supported on the Plugin, since the known limitations of defaulting and admission would not be addressed.

```yaml
apiVersion: greenhouse.sap/v1alpha1
kind: PluginPreset
metadata:
name: cert-manager
spec:
clusterSelector: {...}
plugin:
...
optionValues: {}
pluginDefinition:
name: cert-manager
version: "^v1.1.x"
releaseNamespace: kube-system
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about introducing a PluginPreset.spec.pluginDefinitionRevisionRef field?

This change would provide more control over PluginDefinition revisions by allowing users to choose between automatic and manual updates:

  • Automatic (updatePolicy: automatic): The Greenhouse controller will update pluginDefinitionRevisionRef.name to the latest available PluginDefinitionRevision automatically.
  • Manual (updatePolicy: manual): Users must manually update pluginDefinitionRevisionRef.name when they want to upgrade.

This ensures that the teams can choose between fully automated rollouts vs manual process.

apiVersion: greenhouse.sap/v1alpha1
kind: PluginPreset
metadata:
  name: cert-manager
spec:
  clusterSelector: {...}
  plugin:
    ...
    optionValues: {}
    pluginDefinition:
      name: cert-manager
    # The "updatePolicy" determines whether the PluginPreset should automatically update 
    # to the latest PluginDefinitionRevision.
    updatePolicy: manual | automatic
    pluginDefinitionRevisionRef: 
      name: cert-manager-zf123 
    # If "updatePolicy" is set to "automatic," the Greenhouse controller will automatically
    # update `pluginDefinitionRevisionRef.name` to the latest PluginDefinitionRevision.
    # If set to "manual," users must manually update `pluginDefinitionRevisionRef.name` 
    # when they want to upgrade.
    releaseNamespace: kube-system

Would love to hear your thoughts on this! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With referring explicit versions it is possible to use semantic version constraints to have automatic updates on minor/patch version upgrades etc.. These versions could also be used with Dependabot to create version update PRs on PluginPresets/Plugins.
It would also be possible to stop automatic updates if the underlying chart receives an major version update.

Using the RevisionRef it will restrict to forward-fix only, no patches possible to previous minor versions. To ensure automatic updates applied in the correct order it is necessary to forbid updating PluginDefinitions to a lower HelmChart version than currently existing in the cluster.
This would automatically perform all updates, also for major versions.

PluginDefinition version RevisionNumber RevisionRef
v1.0.1 1 cert-manager-zf123
v2.0.1 2 cert-manager-zf345
v1.0.3 3 cert-manager-ab123

Using RevisionRefs to refer to a particular PluginDefinitionRevision will require manual updates in the PluginPreset/Plugin that are stored in a Git Repository, or in the UI.
It would still be required to have the versions on the PluginDefinitions to be able to easily compare changes between Plugin versions.

@abhijith-darshan
Copy link
Contributor

abhijith-darshan commented Mar 5, 2025

as a first proposal for the PluginCatalog we could do -

apiVersion: greenhouse.sap/v1alpha1
kind: PluginCatalog
metadata:
  name: catalog
  namespace: <org-name>
spec:
  sources:
    - github:
        repoURL: https://github.com/cloudoperators/greenhouse
        secretRef:
          name: github-token
          key: github
          mapping:
            token: token
            appID: appID
            privateKey: privateKey
        targetRevision: main
        path: perses/plugindefinition.yaml # (Explicit path to plugin definition file or can it be directory containing plugin definitions and controller parses all yamls in the base of the dir and extracts all plugin definitions?)
        paths:
          - perses/plugindefinition.yaml
          - alerts/plugindefinition.yaml
          - exposed-services/plugindefinition.yaml
        syncOptions:
          syncInterval: 1h # (How often to sync the catalog with the source)
          automated: true # (Automatically sync the catalog with the source) (mutually exclusive with syncInterval - if syncInterval is set, automated is ignored)
          prune: true # (Remove plugindefinitions that are not in the source) (default: false)

## Do we support buckets?? Many buckets (s3, GCS) support version number or timestamp or generation number, we can use that to check for sync

In target revision we can do -

Pinning Strategies

  • branch name,
  • commit hash,
  • v tag

Watch Strategies on tags or releases

  • v1.x.x

@IvoGoman IvoGoman force-pushed the feat/pluginrelease branch from e6c7aa3 to af9657d Compare March 7, 2025 17:17
@IvoGoman
Copy link
Contributor Author

as a first proposal for the PluginCatalog we could do -

apiVersion: greenhouse.sap/v1alpha1
kind: PluginCatalog
metadata:
  name: catalog
  namespace: <org-name>
spec:
  sources:
    - github:
        repoURL: https://github.com/cloudoperators/greenhouse
        secretRef:
          name: github-token
          key: github
          mapping:
            token: token
            appID: appID
            privateKey: privateKey
        targetRevision: main
        path: perses/plugindefinition.yaml # (Explicit path to plugin definition file or can it be directory containing plugin definitions and controller parses all yamls in the base of the dir and extracts all plugin definitions?)
        paths:
          - perses/plugindefinition.yaml
          - alerts/plugindefinition.yaml
          - exposed-services/plugindefinition.yaml
        syncOptions:
          syncInterval: 1h # (How often to sync the catalog with the source)
          automated: true # (Automatically sync the catalog with the source) (mutually exclusive with syncInterval - if syncInterval is set, automated is ignored)
          prune: true # (Remove plugindefinitions that are not in the source) (default: false)

## Do we support buckets?? Many buckets (s3, GCS) support version number or timestamp or generation number, we can use that to check for sync

In target revision we can do -

Pinning Strategies

  • branch name,
  • commit hash,
  • v tag

Watch Strategies on tags or releases

  • v1.x.x

This PR is concerning the lifecycle, could you add your comment to #20 instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants