Introduce Custom StateCheck Paths in the Module Template

[jakobmoellerdev]

In the current operator design, we are relying on status.state as a field reference. To make the feature a bit more flexible, we want to make sure that we can customise the status.state field with more verification.

We could introduce more state check layers with the following enhancements:

• Introduce the custom state check struct as described in the proposal
• If multiple mappings are present for one state use AND operator for all those mappings
• Document new feature

[kyma-bot]

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]

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]

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

[pbochynski]

I would like to use native IstioOperator as a module in Kyma. This is a sample custom resource for IstioOperator:

apiVersion: install.istio.io/v1alpha1
kind: IstioOperator
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"install.istio.io/v1alpha1","kind":"IstioOperator","metadata":{"annotations":{},"name":"example-istiocontrolplane","namespace":"istio-system"},"spec":{"profile":"demo"}}
  creationTimestamp: "2023-04-06T14:48:14Z"
  finalizers:
  - istio-finalizer.install.istio.io
  generation: 1
  name: example-istiocontrolplane
  namespace: istio-system
  resourceVersion: "2170"
  uid: bab1c58f-c4bd-4d15-9f35-e5a4a6ba2714
spec:
  profile: demo
status:
  componentStatus:
    Base:
      status: HEALTHY
    EgressGateways:
      status: HEALTHY
    IngressGateways:
      status: HEALTHY
    Pilot:
      status: HEALTHY
  status: HEALTHY

I can verify if it is successfully installed using this command:

kubectl wait --for=jsonpath='{.status.status}'=HEALTHY istiooperator/example-istiocontrolplane -n istio-system

I expect that I can put in the ModuleTemplate spec such entry:

  stateCheck: 
     jsonPath: '.status.status'
     value: 'HEALTHY'

For current implementation we can have:

  stateCheck: 
     jsonPath: '.status.state'
     value: 'Ready'

Of course the syntax is just example. You can come with your own proposal.

[janmedrek]

User documentation should be provided for this, at least the simple one pointing out which fields to use and some example values.

[jeremyharisch]

Technical Reference Guide should be put here: https://github.com/kyma-project/lifecycle-manager/blob/main/docs/technical-reference/api/moduleTemplate-cr.md

[jeremyharisch]

#646 is going to be reverted due to wrong implementation: #691

Draft PR for Documentation: #690

[ruanxin]

Proposal:

For 3rd party operator state report, by experiment some operators, I think only mapping the ready state is not enough, It's better to introduce another way to at least map error state, so that if CR get into an unexpected error state, the state can be transmitted into Manifest CR to be notified.

A real-world example, elasticsearch operator have red, green, yellow, unknown to indicate different states in status.health.
https://github.com/elastic/cloud-on-k8s/blob/e3fa2fad671ed9105164df0966ce1dfae286ead0/pkg/apis/elasticsearch/v1/status.go#L20-L25

If we only map green as ready, there is no good way to report other state, if we simply say they are all error states, it might spam us to react on unnecessary state checks, for example: during provision, the state will change from other state to green, also it might temporary went into some other state due to resource changes.

Option 1:

Treat 3rd party operator non ready state as warning?

Option 2:

extend state check to be able to map error state, new format can be:

 stateCheck:
     ready:
         jsonPath: 'status.health'
         value: 'green'
     error:
         jsonPath: 'status.health'
         value: 'red'

Need feedback/opinion for a decision here @pbochynski @janmedrek

Update

 stateCheck:
     -   jsonPath: 'status.health'
         value: 'green'
         mappedState: 'ready'
     -   jsonPath: 'status.health'
         value: 'red'
         mappedState: 'error'
    -   jsonPath: 'status.availableInstance'
         value: '1'
         mappedState: 'ready'

[pbochynski]

I would go with option 2. But you also make clear what is the priority (if both ready and error are true who wins). All states should be configurable. If none of them are true it should be clear what is the default (processing probably is the best candidate)