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

Extend module template with the managed CRDs #963

Closed
1 task
Tracked by #18318
pbochynski opened this issue Oct 19, 2023 · 9 comments
Closed
1 task
Tracked by #18318

Extend module template with the managed CRDs #963

pbochynski opened this issue Oct 19, 2023 · 9 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@pbochynski
Copy link
Contributor

Description

We need the information about CRDs installed by the module for Kyma Dashboard and Kyma CLI. The information can be extracted from the module manifest, but it is not always the complete list (sometimes CRDs are installed by the module operator). It makes sense to have an explicit list of CRDs in the module template spec.

Reasons

No response

Acceptance Criteria

  • module template contains list of managed CRDs

Feature Testing

No response

Testing approach

No response

Attachments

No response

@pbochynski pbochynski added the kind/feature Categorizes issue or PR as related to a new feature. label Oct 19, 2023
@ruanxin
Copy link
Contributor

ruanxin commented Oct 23, 2023

one proposal:

To support the list of managed CRDs, can we put it into some resources, for example, a configmap, or even a dedicated CR. Then LM can deploy it to SKR as a normal module resource during module enabling.

The CLI, busola can use this configmap to cleanup blocking resources during module deleting.

If module is stuck under delete, anyway LM blocks all the module resources to be deleted, so this configmap will present in SRK before module CR get deleted.

@pbochynski
Copy link
Contributor Author

one proposal:

To support the list of managed CRDs, can we put it into some resources, for example, a configmap, or even a dedicated CR. Then LM can deploy it to SKR as a normal module resource during module enabling.

I was thinking about adding that information to the module template spec. We can add it to module metadata and then CLI can pass it to the module template. No work for KLM. Just one additional field to process by Kyma CLI create module command. What do you think?

@ruanxin
Copy link
Contributor

ruanxin commented Oct 25, 2023

Hi @pbochynski ,

thanks for the answer, actually, I have some concerns if it's good to put this information in module template.

  1. The remote module template at the moment can be immediately removed if delete kyma triggered by kap operator, there is no dependency on it, but if we put this information inside module template, then it means LM should wait for all module get deleted successfully before remove module template. The current logic needs to be changed. But if it's a module resources, anyway, the lifetime of this data is bound to the module, a bit more reliable. btw, is it a valid use case that when kyma is under deleting, we need to use this information?

  2. change module template spec requires a solid definition for the format of this data, at the moment, it's just CRD, probably, with GVK information is fine, but later, if need to add more concrete resources, for example, support label for specific resources, it needs to change again. But this is resolvable, we can define this field as raw json data, and extend the unmarshal logic in the CLI and busola later.

  3. I second you about add it to module metadata, as annotation, then it's more flexible, we just need to be aware of the size limit, but annotation is 265kb, should be enough.

If point 1 is irrelevant, I think we can go with your idea.

@pbochynski
Copy link
Contributor Author

I think introducing another config map or another CR for describing module is not the best approach. Let me use the example. Right now module contributors have to provide metadata (module-config.yaml). This is the config for serverless with additional CRD list:

name: kyma-project.io/module/serverless
channel: fast
version: v1.0.3
manifest: serverless-operator.yaml
defaultCR: default-serverless-cr.yaml
beta: false
labels:
  "operator.kyma-project.io/controller-name": "manifest"
  "operator.kyma-project.io/managed-by": "lifecycle-manager"
annotations:
  "operator.kyma-project.io/doc-url": "https://kyma-project.io/#/serverless-manager/user/README"
moduleRepo: https://github.com/kyma-project/serverless-manager.git
managedResources:
  - kind: Function
    group: serverless.kyma-project.io
    version: v1alpha2
  - kind: Serverless  
    group: operator.kyma-project.io
    version: v1alpha1

which will produce such module template:

apiVersion: operator.kyma-project.io/v1beta2
kind: ModuleTemplate
metadata:
  name: serverless-fast
  namespace: kcp-system
  labels:
    "operator.kyma-project.io/controller-name": "manifest"
    "operator.kyma-project.io/managed-by": "lifecycle-manager"
    "operator.kyma-project.io/module-name": "serverless"
  annotations:
    "operator.kyma-project.io/doc-url": "https://kyma-project.io/#/serverless-manager/user/README"
    "operator.kyma-project.io/is-cluster-scoped": "false" 
spec:
  channel: fast 
  data:
    apiVersion: operator.kyma-project.io/v1alpha1
    kind: Serverless
    metadata:
      name: default
    spec:
      dockerRegistry:
        enableInternal: true
  managedResources:
    - kind: Function
      group: serverless.kyma-project.io
      version: v1alpha2
    - kind: Serverless  
      group: operator.kyma-project.io
      version: v1alpha1
  descriptor:
     ...

Now about your concerns above.

  1. LifecycleManager is not processing that information at all. It is just the information for Kyma dashboard (or kyma CLI) which resources should be removed if force flag is enabled for module deletion.
  2. ModuleTemplate is already returned as json from API server. Busola can read values directly and pass them in the delete request to the API server. This extension of spec is backward compatible and if we need more info we need to introduce it in the backward compatible way. Anyway only we produce module templates so even incompatible changes can be addressed.
  3. Annotations will be more awkward and harder to parse by clients IMHO.

@pbochynski
Copy link
Contributor Author

@valentinvieriu You probably should tell us your preference, as you are a customer of this feature :)

@ruanxin
Copy link
Contributor

ruanxin commented Oct 25, 2023

Hi @pbochynski,

  1. LifecycleManager is not processing that information at all. It is just the information for Kyma dashboard (or kyma CLI) which resources should be removed if force flag is enabled for module deletion.

I got your point, I think I need to clarify the use case I'm concerned about more clearly. The use case is when Kyma CR is under deletion, the ModuleTemplate which synced to remote SKR cluster will be deleted, in this case, there is no moduletemplate for Kyma dashboard (or kyma CLI) to consume with. So the question is during cluster deprovision, is it necessary to let customers still possible to use some tools like Kyma CLI to interactive cluster to force delete module so that cluster can be deprovision faster, maybe it's not needed, eventually the cluster should be deleted automatically.

tbh, another reason I'm not telling is, actually I plan to convince you to stop module template sync in the future, but this is just an immature idea atm. I see this module template which synced in remote cluster, they just serve the feature:

  1. to offer some module metadata, in this case, most of the information in OCM descriptor is irrelevant to the customer. So why not introduce a metadata resource (e.g: a configmap) into the module manifest yaml. This way, LM could deploy and sync it into the SKR cluster, just like any other module resource.
  2. let SKR cluster possible to see a list of module which possible to be enabled with, but for this use case, actually, we could just sync one resource which contains a list of possible modules into remote skr, but don't need to sync all module templates. If this change is made, we can simplify the template sync logic a lot and also improve the performance of LM, and reduce the network traffic (just need to sync one resource instead of tens of moduletemplates).

However, introducing more responsibilities into the module template basically makes this option impossible.

@pbochynski
Copy link
Contributor Author

Kyma deprovisioning is something different. If Kyma is deprovisioned we already have a logic to clean up all resources in the SKR.

Another topic is what should be synced to the cluster. I see potential optimization in making the module template smaller (skip some irrelevant OCM fields) or consolidating it into one resource. But for me it is just optimization. We need a list of available modules with their metadata in the target cluster. Right now for that purpose, we have ModuleTemplate CRs. I am not sure if replacing that list with single resource is going to change the KLM performance significantly. If you say that it reduces reconciliation time more than 20% we should start such initiative.
Converting custom resource to config map does not bring any value in my opinion (with CR we get schema and validation)

@kyma-bot
Copy link
Contributor

This issue or PR has been automatically marked as stale due to the lack of recent activity.
Thank you for your contributions.

This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied
  • After 7d of inactivity since lifecycle/stale was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Close this issue or PR with /close

If you think that I work incorrectly, kindly raise an issue with the problem.

/lifecycle stale

@kyma-bot kyma-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 25, 2023
@janmedrek janmedrek removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 28, 2023
@janmedrek
Copy link
Contributor

As we talked with @pbochynski there are two options for determining a list of managed CRDs: #1211 (comment)

For now, I will close this issue, if we decide to go that way we will reopen it and continue the work here.

@janmedrek janmedrek closed this as not planned Won't fix, can't repro, duplicate, stale Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants