Allow empty backend #274

muralov · 2023-11-30T08:12:50Z

Description
Allow empty backend and set warning state without doing anything

Changes proposed in this pull request:

Allow empty backend by removing backend defaulting
- Set warning state and do nothing
Set warning state if Event Mesh secret is missing
Set warning state if NATS module is missing
Forbid deleting backend config

Related issue(s)
#266

NHingerl · 2023-12-04T12:28:10Z

docs/user/02-configuration.md

+| **backend**  | object | Backend defines the active backend used by Eventing. |
+| **backend.&#x200b;config**  | object | Config defines configuration for eventing backend. |
+| **backend.&#x200b;config.&#x200b;domain**  | string | Domain defines the cluster public domain used to configure the EventMesh Subscriptions and their corresponding ApiRules. |
+| **backend.&#x200b;config.&#x200b;eventMeshSecret**  | string | EventMeshSecret defines the namespaced name of K8s Secret containing EventMesh credentials. The format of name is "namespace/name". |


I don't understand this sentence. What is a "namespaced name"?

NHingerl · 2023-12-04T12:28:28Z

docs/user/02-configuration.md

+| **backend.&#x200b;config**  | object | Config defines configuration for eventing backend. |
+| **backend.&#x200b;config.&#x200b;domain**  | string | Domain defines the cluster public domain used to configure the EventMesh Subscriptions and their corresponding ApiRules. |
+| **backend.&#x200b;config.&#x200b;eventMeshSecret**  | string | EventMeshSecret defines the namespaced name of K8s Secret containing EventMesh credentials. The format of name is "namespace/name". |
+| **backend.&#x200b;config.&#x200b;eventTypePrefix**  | string |  |


no description for this one?

NHingerl · 2023-12-04T12:32:15Z

docs/user/02-configuration.md

+| **publisher.&#x200b;replicas.&#x200b;max**  | integer | Max defines maximum number of replicas. |
+| **publisher.&#x200b;replicas.&#x200b;min**  | integer | Min defines minimum number of replicas. |
+| **publisher.&#x200b;resources**  | object | Resources defines resources for eventing-publisher-proxy. |
+| **publisher.&#x200b;resources.&#x200b;claims**  | \[\]object | Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container. 


This isn't rendered correctly when the sentences are in different lines:

The table is auto-generated, so you might have to adjust the formatting somewhere else instead of here. But here's how it should look like eventually:

Suggested change

| **publisher.resources.claims** | \[\]object | Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container.

| **publisher.resources.claims** | \[\]object | Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container. This is an alpha field and requires enabling the DynamicResourceAllocation feature gate. This field is immutable. It can only be set for containers. |

NHingerl · 2023-12-04T12:34:43Z

docs/user/02-configuration.md

+| Parameter | Type | Description |
+| ---- | ----------- | ---- |
+| **activeBackend** (required) | string |  |
+| **conditions**  | \[\]object | Condition contains details for one aspect of the current state of this API Resource. --- This struct is intended for direct use as an array at the field path .status.conditions.  For example, 


Same here, with the content spread across three lines, this table cell isn't rendered correctly:

NHingerl · 2023-12-04T12:36:33Z

docs/user/02-configuration.md

+| **conditions**  | \[\]object | Condition contains details for one aspect of the current state of this API Resource. --- This struct is intended for direct use as an array at the field path .status.conditions.  For example, 
+ type FooStatus struct{ // Represents the observations of a foo's current state. // Known .status.conditions.type are: "Available", "Progressing", and "Degraded" // +patchMergeKey=type // +patchStrategy=merge // +listType=map // +listMapKey=type Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"` 
+ // other fields } |
+| **conditions.&#x200b;lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another. This should be when the underlying condition changed.  If that is not known, then using the time when the API field changed is acceptable. |


"This should be when"

Maybe a word is missing here? Not sure what this means.

NHingerl · 2023-12-04T12:38:34Z

docs/user/02-configuration.md

+ // other fields } |
+| **conditions.&#x200b;lastTransitionTime** (required) | string | lastTransitionTime is the last time the condition transitioned from one status to another. This should be when the underlying condition changed.  If that is not known, then using the time when the API field changed is acceptable. |
+| **conditions.&#x200b;message** (required) | string | message is a human readable message indicating details about the transition. This may be an empty string. |
+| **conditions.&#x200b;observedGeneration**  | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. |


Suggested change

| **conditions.observedGeneration** | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. |

| **conditions.observedGeneration** | integer | observedGeneration represents the .metadata.generation that the condition was set based upon. For example, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. |

Fix it in the source file, not here :)

Make state warning and BackendAvailable condition false for empty backend config

* Set Warning state if EventMes secret missing * Fix failing tests

It is not allowed to delete existing backend config, but evneting CR can be created with empty backend config.

Create integration tests for backend config deletion validation

Auto generation of doc is breaking the documentation

muralov · 2023-12-07T06:09:13Z

@NHingerl I've applied my only change to the .md file and reverted the auto-generated parts as it breaks the doc.

.golangci.yaml

internal/controller/operator/eventing/eventmesh.go

muralov requested review from a team as code owners November 30, 2023 08:12

muralov requested a review from marcobebway November 30, 2023 08:12

kyma-bot added area/documentation Issues or PRs related to documentation cla: yes Indicates the PR's author has signed the CLA. labels Nov 30, 2023

muralov added area/eventing Issues or PRs related to eventing kind/feature Categorizes issue or PR as related to a new feature. labels Nov 30, 2023

kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 30, 2023

muralov added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 30, 2023

muralov force-pushed the allow-empty-backend branch from ab6908e to 443bf68 Compare November 30, 2023 08:29

kyma-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 30, 2023

NHingerl reviewed Dec 4, 2023

View reviewed changes

muralov changed the title ~~Allow empty backend~~ WIP: Allow empty backend Dec 4, 2023

kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 4, 2023

muralov force-pushed the allow-empty-backend branch 5 times, most recently from 0faf097 to 8417d28 Compare December 6, 2023 05:38

muralov added 3 commits December 6, 2023 06:39

Allow empty backend w/o defaulting

f2c3760

Set warning state for empty backend

1da00f9

Make state warning and BackendAvailable condition false for empty backend config

Set Warning if NATS module is missing

17ad780

muralov force-pushed the allow-empty-backend branch from 8417d28 to fb10b89 Compare December 6, 2023 05:41

Set Warning state if EventMes secret missing

32cce57

* Set Warning state if EventMes secret missing * Fix failing tests

muralov force-pushed the allow-empty-backend branch from fb10b89 to 32cce57 Compare December 6, 2023 05:47

Fix linting errors

f655f2c

muralov removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2023

muralov added 2 commits December 6, 2023 14:25

Forbid deleting backend configuration

6eace18

It is not allowed to delete existing backend config, but evneting CR can be created with empty backend config.

Revert golangci-lint configuration

3c10365

muralov changed the title ~~WIP: Allow empty backend~~ Allow empty backend Dec 6, 2023

kyma-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2023

Implement int tests

73de8c3

Create integration tests for backend config deletion validation

muralov linked an issue Dec 6, 2023 that may be closed by this pull request

Allow empty backend-spec on eventing-cr #266

Closed

6 tasks

Revert the documentation

3f26c86

Auto generation of doc is breaking the documentation

muralov force-pushed the allow-empty-backend branch from 52caff4 to 3f26c86 Compare December 7, 2023 06:07

Move Retrieving EventMesh Secret

d105052

marcobebway reviewed Dec 7, 2023

View reviewed changes

.golangci.yaml Outdated Show resolved Hide resolved

NHingerl previously approved these changes Dec 7, 2023

View reviewed changes

kyma-bot assigned NHingerl Dec 7, 2023

kyma-bot added the lgtm Looks good to me! label Dec 7, 2023

marcobebway reviewed Dec 7, 2023

View reviewed changes

internal/controller/operator/eventing/eventmesh.go Outdated Show resolved Hide resolved

Improve for review comments

5eb1206

muralov dismissed NHingerl’s stale review via 5eb1206 December 7, 2023 09:41

kyma-bot removed the lgtm Looks good to me! label Dec 7, 2023

marcobebway approved these changes Dec 7, 2023

View reviewed changes

kyma-bot assigned marcobebway Dec 7, 2023

kyma-bot added the lgtm Looks good to me! label Dec 7, 2023

NHingerl approved these changes Dec 7, 2023

View reviewed changes

kyma-bot merged commit e4b79e4 into kyma-project:main Dec 7, 2023
10 checks passed

grischperl mentioned this pull request Jan 14, 2024

Fix defaulting rules to set defaults depending on NATS or EventMesh backend #80

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow empty backend #274

Allow empty backend #274

muralov commented Nov 30, 2023 •

edited

Loading

NHingerl Dec 4, 2023

NHingerl Dec 4, 2023

NHingerl Dec 4, 2023

NHingerl Dec 4, 2023

NHingerl Dec 4, 2023

NHingerl Dec 4, 2023

muralov commented Dec 7, 2023

	\| publisher.resources.claims \| \[\]object \| Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container.
	\| publisher.resources.claims \| \[\]object \| Claims lists the names of resources, defined in spec.resourceClaims, that are used by this container. This is an alpha field and requires enabling the DynamicResourceAllocation feature gate. This field is immutable. It can only be set for containers. \|

	\| conditions.observedGeneration \| integer \| observedGeneration represents the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. \|
	\| conditions.observedGeneration \| integer \| observedGeneration represents the .metadata.generation that the condition was set based upon. For example, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. \|

Allow empty backend #274

Allow empty backend #274

Conversation

muralov commented Nov 30, 2023 • edited Loading

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

NHingerl Dec 4, 2023

Choose a reason for hiding this comment

muralov commented Dec 7, 2023

muralov commented Nov 30, 2023 •

edited

Loading