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

Update recipe CRD to match with other consumers #35

Merged
merged 10 commits into from
Sep 18, 2024

Conversation

raghavendra-talur
Copy link
Member

@raghavendra-talur raghavendra-talur commented Sep 13, 2024

This pull request introduces several modifications to the API to have better compatibility with other consumers of the recipe CRD. The key changes include:

  1. Command and Timeout Changes:

    • The command is now a single string format to improve compatibility with external recipe consumers.
    • The timeout field has been changed to an integer type for better consistency.
  2. Hooks Enhancements:

    • Removed validation for resource types in hooks to facilitate the introduction of additional hook types.
    • Made the namespace field mandatory for hooks.
  3. Group Configuration Updates:

    • Introduced new fields in Groups, including:
      • RestoreOverwriteResources: Allows resource overwriting during restores.
      • RestoreGroupStatus: Enables status section restoration in Custom Resources (CRs).
      • ExcludedNamespaces: Specifies namespaces to be excluded during operations.
      • IncludedNamespacesByLabel: Supports namespace selection based on label selectors.
  4. Workflow Improvements:

    • The previous capture and recover workflow has been removed and replaced with a more streamlined list of workflows, specifically backup and restore, which are now associated with default operations.
  5. Additional Changes:

    • Added generated files as part of the updates.

Co-authorship: This pull request includes contributions from Jose A. Rivera.

raghavendra-talur and others added 10 commits September 16, 2024 16:32
These are replaced by a list of workflows. The workflow names
* backup
* restore

have special meaning. They will be used by default backup/capture and
restore/recover workflow.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
This allows for selection of namespaces by a label selector.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
When set, this allows for restoring of the status section in the CRs.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
When set, the resources are overwritten when a restore is being
performed.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
This is in preparation to introduction of other type of hooks.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
This is for compatibility with other consumers of recipes.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
This is for compatibility with other consumers of recipes.

Signed-off-by: Raghavendra Talur <[email protected]>
Co-Authored-by: Jose A. Rivera <[email protected]>
Signed-off-by: Raghavendra Talur <[email protected]>
@ShyamsundarR
Copy link
Member

LGTM, will wait for other reviews and merge as appropriate.

// List of namespaces to include.
//+optional
IncludedNamespaces []string `json:"includedNamespaces,omitempty"`
// List of namespace to exclude
ExcludedNamespaces []string `json:"excludedNamespaces,omitempty"`
Copy link

@asn1809 asn1809 Sep 17, 2024

Choose a reason for hiding this comment

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

Can we also add excludedNamespacesByLabel?

Copy link
Contributor

Choose a reason for hiding this comment

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

We thought about it, but given that other consumers haven't implemented a excludedNamespacesByLabel it could be that there's no demand for it, or that there exist other complications that are not immediately apparent before attempting implementation. Given that it's always easier to add rather than remove fields from an API, it feels prudent to leave it out until there's an explicit request for it.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this PR I am only trying to match the CRD with other consumers expectations. We can add other features in a subsequent PR.

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

Successfully merging this pull request may close these issues.

4 participants