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

Delete modes for modules #1211

Closed
Tracked by #18450
janmedrek opened this issue Dec 29, 2023 · 10 comments
Closed
Tracked by #18450

Delete modes for modules #1211

janmedrek opened this issue Dec 29, 2023 · 10 comments
Labels
Epic kind/feature Categorizes issue or PR as related to a new feature.

Comments

@janmedrek
Copy link
Contributor

janmedrek commented Dec 29, 2023

Description

Kyma Lifecycle Manager should support two different cases for the deletion of modules:

  • Regular delete - a bit modified version of current behaviour
  • Ignore - module is just dropped from the Kyma CR and no further action is taken

This supports use cases mentioned in this proposal, such as cross-use of OS and managed modules, adapting configuration and the cleanup behaviour desired by the user.

The majority of the scenarios depend on #963 as information on which the module manages resources is needed.

As for the detailed behaviour of the deletion modes:

  • Regular delete - this is a blocking deletion scenario in which KLM should monitor whether there are any managed resources left.
    • If there are none - it should remove the operator and CRDs
    • If there are any - wait until the resources are gone
  • Ignore - in this mode there is no action taken by the KLM, the module is just removed from Kyma CR and should no longer be reconciled by the KLM (Manifest CR can be removed)

This should be done on the per-module level, different modules in one Kyma should be able to have different deletion modes.

Example:

  • Kyma A
    • Serverless A with blocking deletion
    • API Gateway A with ignore deletion
  • Kyma B
    • Serverless B with blocking deletion
    • API Gateway B with blocking deletion

Reasons

With the reasoning behind kyma-project/community#870, there comes a requirement to adapt to different use cases for the Kyma usages. We need to support different scenarios of cross-use of OS and managed modules, configurations and cleanups.

The deletion logic gets convoluted and it would be desired to split it into three different, transparent modes.use cases

Acceptance Criteria

  • Describe the Kyma Module deletion behaviour
    • Given a Kyma Module
      • And a SKR Kyma CR is created on the SKR Cluster
      • And a Manifest CR is created on the KCP Cluster
    • When the Deletion is requested for the Kyma Module
      • Then the Module Domain CRs are being monitored
        • And the Module Operator and Module Domain CRDs are not deleted if there are any Module Domain CRs
        • And the Module Operator and Module Domain CRDs are deleted if all Module Domain CRs are gone
    • When the Unmanage is requested for the Kyma Module
      • Then the Kyma Module entry is removed from the SKR Kyma CR
        • Kyma Module is no longer reconciled
        • Manifest CR is removed from the KCP Cluster
        • Module Operator, Module CR, Module Domain CRDs, and Module Domain CRs are kept intact in the SKR Cluster
@janmedrek janmedrek added kind/feature Categorizes issue or PR as related to a new feature. Epic labels Dec 29, 2023
@pbochynski
Copy link
Contributor

In my opinion force delete should be executed by the user using UI/CLI. User should be informed about the list of managed resources that will be deleted to make a conscious decision. The only situation where force deletion can be done without user interaction should be Kyma runtime deprovisioning. But that part can be done by a separate process.

@janmedrek
Copy link
Contributor Author

When it comes to determining which resources are "managed" there are two approaches:

The second option requires a bit more effort in the implementation part, but in the end - provides a simpler CRD design.

@janmedrek
Copy link
Contributor Author

Issue for "unmanaging" the modules: #1427

@jeremyharisch
Copy link
Contributor

jeremyharisch commented May 8, 2024

Acceptance Criteria for EPIC:

  • Research about the topic
  • Think about a proper issue split for this EPIC
  • Create Sub-Issues to cover the implementation
  • Regarding business requirements talk to @janmedrek if needed

Timebox: 2 Days

@nesmabadr
Copy link
Contributor

nesmabadr commented May 28, 2024

We agreed on the following:

  • The default value for DeletionMode would be "Ignore"
  • As a transition period, we will not have a default value for the deletionMode for sometime until the users are conscious about this value. Throughout this period, if the deletion mode is empty, we will handle the deletion as is now i.e delete the module CR first before the rest
  • The CustomResourcePolicy will only control the creation of the module CR by the Lifecycle manager, thus we will rename the "CreateAndDelete" option to "Create" only to reflect its actual usage. (This will happen in the new API version, for now we will add the "Create" as a third option)
  • The deletionMode in the Manifest will be retrieved from the KymaStatus ModuleStatus when the module is disabled in the Kyma CR (to try to have the latest mode before deletion in order to minimize the possibility of the corner case mentioned here from happening)

@janmedrek
Copy link
Contributor Author

Image

@ruanxin
Copy link
Contributor

ruanxin commented May 29, 2024

@janmedrek , I think this epic can be converted together with that "unmanaging" the modules: #1427.

Basically, this "Ignore" mode is regarding how KLM deal with those modules switch by user from "managed" module to "unmanaged", right? strictly speaking, it's not deleting, for deleting module, KLM should always use blocking mode.

Then I aligned with your proposal, introducing managed field to spec.modules makes more sense, and for those modules to become "unmanaged", they still keep in the spec.modules, KLM just take care of force deleting related manifest CR. But for module removed from spec.modules, it has the meaning to indeed delete this module by KLM with blocking strategy.

Pros:

  • we keep the existing behaviour regarding remove module from spec.modules clearly on the implementation level, just need to extend to support watching managed resources.

  • avoid implementing solution to keep this deletionMode into other places. With current approach, it persists in spec.modules field, but when user deletes module, this entry will be removed, KLM have no way to detect which deletionMode.

@ruanxin
Copy link
Contributor

ruanxin commented May 31, 2024

@janmedrek, regarding Default CR Strategy, the Ignore mode, as I mentioned in daily, we currently have bug which mentioned by PB in his proposal point 4:

With Ignore strategy KLM should not delete module deployment as it can lead to orphan resources with misleading status (module config with state Ready, but operator undeployed). With CreateAndDelete strategy the deletion process should be done with blocking strategy (do not delete operator manifest before all managed resources are gone - point no 2).

To make it clear, current Ignore strategy, as you mentioned "Default CR not removed when the module is disabled", is not a solid solution, when module is disabled, the module operator will be remove, if the Default CR is not removed, then there is no operator update it's status.

Please create one issue related to this problem, and clarify following questions:

  1. Does it means for the fixed Ignore strategy, the solution is KLM does not delete any module resources?
  2. Then what's the different between your IgnoreDeletion module deletion strategy with this fixed Ignore strategy?

@janmedrek
Copy link
Contributor Author

@ruanxin thanks for the insights.

From that point of view, it does not make sense to keep the current "ignore" strategy.
Let us align on this tomorrow and I will update the tickets. Most likely we will keep the current blocking strategy for deletion and convert this to the "unmanage" ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

5 participants