Set selector and label specific to platform service #341

masayag · 2024-01-01T16:19:40Z

Description of the change:
Set a specific selector for each service installed by the platform.

Closes #340

Motivation for the change:
Having the same selector will cause issues with the service routing to the incorrect pod and also for issues with scaling.

Checklist

Add or Modify a unit test for your change
Have you verified that tall the tests are passing?

How to backport a pull request to a different branch?

In order to automatically create a backporting pull request please add one or more labels having the following format backport-<branch-name>, where <branch-name> is the name of the branch where the pull request must be backported to (e.g., backport-7.67.x to backport the original PR to the 7.67.x branch).

NOTE: backporting is an action aiming to move a change (usually a commit) from a branch (usually the main one) to another one, which is generally referring to a still maintained release branch. Keeping it simple: it is about to move a specific change or a set of them from one branch to another.

Once the original pull request is successfully merged, the automated action will create one backporting pull request per each label (with the previous format) that has been added.

If something goes wrong, the author will be notified and at this point a manual backporting is needed.

NOTE: this automated backporting is triggered whenever a pull request on main branch is labeled or closed, but both conditions must be satisfied to get the new PR created.

jordigilh · 2024-01-02T16:00:32Z

/lgtm

jordigilh · 2024-01-02T16:01:50Z

@ricardozanini PTAL, @masayag found an issue where the DI and JS services contain the same endpoints because the label selector resolves to the DI and JS pods.

$> kubectl get svc,ep,pod -n test-898 -o wide  
NAME                                             TYPE        CLUSTER-IP       EXTERNAL-IP   PORT(S)   AGE     SELECTOR
service/sonataflow-platform-data-index-service   ClusterIP   10.107.167.201   <none>        80/TCP    2m47s   app=sonataflow-platform
service/sonataflow-platform-jobs-service         ClusterIP   10.98.144.50     <none>        80/TCP    2m47s   app=sonataflow-platform

NAME                                               ENDPOINTS                             AGE
endpoints/sonataflow-platform-data-index-service   10.244.1.172:8080,10.244.1.173:8080   2m47s
endpoints/sonataflow-platform-jobs-service         10.244.1.172:8080,10.244.1.173:8080   2m47s

NAME                                                          READY   STATUS      RESTARTS   AGE     IP             NODE       NOMINATED NODE   READINESS GATES
pod/sonataflow-platform-cache                                 0/1     Completed   0          3m7s    10.244.1.171   minikube   <none>           <none>
pod/sonataflow-platform-data-index-service-8674cf59d4-hqfhx   1/1     Running     0          2m47s   10.244.1.173   minikube   <none>           <none>
pod/sonataflow-platform-jobs-service-5fbd6f79cd-sgvq4         1/1     Running     0          2m47s   10.244.1.172   minikube   <none>           <none>

jordigilh · 2024-01-02T22:09:31Z

@wmedvede PTAL.

tchughesiv · 2024-01-08T15:15:52Z

controllers/platform/k8s.go

@@ -196,7 +196,7 @@ func createDeployment(ctx context.Context, client client.Client, platform *opera

 func createService(ctx context.Context, client client.Client, platform *operatorapi.SonataFlowPlatform, ps services.Platform) error {
 	lbl := map[string]string{
-		workflowproj.LabelApp: platform.Name,
+		workflowproj.LabelApp: ps.GetServiceName(),


rather than change the existing label, we should probably create an additional label specific to the service.

Should we maintain the same label for both services?

By looking at the previous deployment of data-index, the label was app.kubernetes.io/name: data-index-service-postgresql
The label for the job-service was app.kubernetes.io/name: jobs-service-postgresql

If we wish to maintain a relationship between the services and the platform, ownerReference seems a better representation of the relations between them.

Raised my concern, but will act based on your decision here.

that may be the case for the tech preview templates, but the operator doesn't set that label. it sets app: <platform.name>. i'm suggesting we leave that label as-is and add a new service-specific label... maybesonataflow.org/service?

so the labels would look like this for the related service objects ... using a SonataflowPlatform named test

data index objects

labels: app: test sonataflow.org/service: test-data-index-service

job service objects

labels: app: test sonataflow.org/service: test-jobs-service

masayag · 2024-01-09T10:40:22Z

With recent version of the PR, it looks like this:

kubectl get svc,ep,pod -n sonataflow-infra -o wide  -l app=sonataflow-platform --show-labels
NAME                                             TYPE        CLUSTER-IP       EXTERNAL-IP   PORT(S)   AGE   SELECTOR                                                        LABELS
service/sonataflow-platform-data-index-service   ClusterIP   172.30.109.125   <none>        80/TCP    34m   sonataflow.org/service=sonataflow-platform-data-index-service   app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-data-index-service
service/sonataflow-platform-jobs-service         ClusterIP   172.30.89.1      <none>        80/TCP    34m   sonataflow.org/service=sonataflow-platform-jobs-service         app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-jobs-service

NAME                                               ENDPOINTS           AGE   LABELS
endpoints/sonataflow-platform-data-index-service   10.133.2.65:8080    34m   app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-data-index-service
endpoints/sonataflow-platform-jobs-service         10.135.0.249:8080   34m   app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-jobs-service

NAME                                                         READY   STATUS    RESTARTS   AGE   IP             NODE                        NOMINATED NODE   READINESS GATES   LABELS
pod/sonataflow-platform-data-index-service-ff4db5cfc-9mb4v   1/1     Running   0          34m   10.133.2.65    ocp413-worker-0.lab.local   <none>           <none>            app=sonataflow-platform,pod-template-hash=ff4db5cfc,sonataflow.org/service=sonataflow-platform-data-index-service
pod/sonataflow-platform-jobs-service-644c744bf7-nmfn2        1/1     Running   0          34m   10.135.0.249   ocp413-worker-2.lab.local   <none>           <none>            app=sonataflow-platform,pod-template-hash=644c744bf7,sonataflow.org/service=sonataflow-platform-jobs-service

tchughesiv

requested one small change... otherwise lgtm

tchughesiv · 2024-01-09T13:40:23Z

controllers/platform/k8s.go

@@ -243,7 +250,7 @@ func createConfigMap(ctx context.Context, client client.Client, platform *operat
 			Name:      ps.GetServiceCmName(),
 			Namespace: platform.Namespace,
 			Labels: map[string]string{
-				workflowproj.LabelApp: platform.Name,
+				workflowproj.LabelService: platform.Name,


shouldn't this have both labels set? also, this line should remain as-is... and the new label added. this should match the other objects.

yes, both labels should be added here.

→ kubectl get cm -n sonataflow-infra -l app=sonataflow-platform --show-labels NAME DATA AGE LABELS sonataflow-platform-data-index-service-props 1 4m9s app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-data-index-service sonataflow-platform-jobs-service-props 1 4m8s app=sonataflow-platform,sonataflow.org/service=sonataflow-platform-jobs-service

Signed-off-by: Moti Asayag <[email protected]>

wmedvede · 2024-01-11T15:01:29Z

E2E tests on this PR are failing because of the java17 vesion fix send in another commit.
I was able to reproduce the failure locally, and also to verify that the test executes well after rebasing with main.
So I'll merge this since we need this fix. Any other tunnings on the lables or selector labels can be done in future PR if needed.

ricardozanini · 2024-01-15T13:38:08Z

workflowproj/operator.go

@@ -34,6 +34,8 @@ const (
 	ApplicationPropertiesFileName = "application.properties"
 	// LabelApp key to use among object selectors, "app" is used among k8s applications to group objects in some UI consoles
 	LabelApp = "app"
+	// LabelService key to use among object selectors
+	LabelService = "sonataflow.org/service"


@masayag can you please fix this in a follow up PR?

LabelService = metadata.Domain + "/service"

@ricardozanini done here: #351

Signed-off-by: Moti Asayag <[email protected]>

masayag requested a review from ricardozanini as a code owner January 1, 2024 16:19

tchughesiv suggested changes Jan 8, 2024

View reviewed changes

masayag force-pushed the platform-service-selector branch from a8caa98 to 5cc9cb6 Compare January 9, 2024 10:36

masayag requested a review from tchughesiv January 9, 2024 10:38

masayag mentioned this pull request Jan 9, 2024

Update orchestrator plugins version to 0.0.6-alpha3 and sonataflow-operator-catalog parodos-dev/orchestrator-helm-chart#24

Merged

tchughesiv suggested changes Jan 9, 2024

View reviewed changes

Set selector and label specific to platform service

96ddcaa

Signed-off-by: Moti Asayag <[email protected]>

masayag force-pushed the platform-service-selector branch from 5cc9cb6 to 96ddcaa Compare January 9, 2024 14:01

tchughesiv approved these changes Jan 9, 2024

View reviewed changes

tchughesiv mentioned this pull request Jan 11, 2024

kogito-serverless-operator-356: Temporary use of the nightly images #347

Merged

2 tasks

wmedvede approved these changes Jan 11, 2024

View reviewed changes

wmedvede merged commit e631455 into apache:main Jan 11, 2024
3 of 4 checks passed

ricardozanini reviewed Jan 15, 2024

View reviewed changes

rgdoliveira pushed a commit to rgdoliveira/kogito-serverless-operator that referenced this pull request Jan 29, 2024

Set selector and label specific to platform service (apache#341)

8c9d7e0

Signed-off-by: Moti Asayag <[email protected]>

rgdoliveira pushed a commit to rgdoliveira/kogito-serverless-operator that referenced this pull request Jan 29, 2024

Set selector and label specific to platform service (apache#341)

9b66f95

Signed-off-by: Moti Asayag <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set selector and label specific to platform service #341

Set selector and label specific to platform service #341

masayag commented Jan 1, 2024

jordigilh commented Jan 2, 2024

jordigilh commented Jan 2, 2024 •

edited

Loading

jordigilh commented Jan 2, 2024

tchughesiv Jan 8, 2024

masayag Jan 8, 2024

tchughesiv Jan 8, 2024 •

edited

Loading

masayag commented Jan 9, 2024

tchughesiv left a comment

tchughesiv Jan 9, 2024

masayag Jan 9, 2024

masayag Jan 9, 2024

wmedvede commented Jan 11, 2024

ricardozanini Jan 15, 2024

masayag Jan 15, 2024

Set selector and label specific to platform service #341

Set selector and label specific to platform service #341

Conversation

masayag commented Jan 1, 2024

jordigilh commented Jan 2, 2024

jordigilh commented Jan 2, 2024 • edited Loading

jordigilh commented Jan 2, 2024

tchughesiv Jan 8, 2024

Choose a reason for hiding this comment

masayag Jan 8, 2024

Choose a reason for hiding this comment

tchughesiv Jan 8, 2024 • edited Loading

Choose a reason for hiding this comment

masayag commented Jan 9, 2024

tchughesiv left a comment

Choose a reason for hiding this comment

tchughesiv Jan 9, 2024

Choose a reason for hiding this comment

masayag Jan 9, 2024

Choose a reason for hiding this comment

masayag Jan 9, 2024

Choose a reason for hiding this comment

wmedvede commented Jan 11, 2024

ricardozanini Jan 15, 2024

Choose a reason for hiding this comment

masayag Jan 15, 2024

Choose a reason for hiding this comment

jordigilh commented Jan 2, 2024 •

edited

Loading

tchughesiv Jan 8, 2024 •

edited

Loading