Add runtime label based on metadata:language/projectType #6112

feloy · 2022-09-12T07:55:07Z

What type of PR is this:

/kind feature

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes #6044

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:

netlify · 2022-09-12T07:55:15Z

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	`6e50b8f`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63208be4e9271d0009e29ddf

odo-robot · 2022-09-12T08:02:45Z

Unit Tests on commit d52f656 finished successfully.
View logs: TXT HTML

odo-robot · 2022-09-12T08:11:41Z

Validate Tests on commit d52f656 finished successfully.
View logs: TXT HTML

odo-robot · 2022-09-12T08:13:22Z

OpenShift Tests on commit d52f656 finished successfully.
View logs: TXT HTML

odo-robot · 2022-09-12T08:17:25Z

Kubernetes Tests on commit d52f656 finished successfully.
View logs: TXT HTML

odo-robot · 2022-09-12T08:26:36Z

Windows Tests (OCP) on commit d52f656 finished successfully.
View logs: TXT HTML

valaparthvi · 2022-09-12T09:01:43Z

pkg/labels/labels.go

+	if runtime != "" {
+		labels[openshiftRunTimeLabel] = runtime
+	}


This is an OpenShift specific label, isn't it? It does not make sense to add it to resources that might be deployed in a K8s cluster. Can we ensure it's only added if the cluster is OC?

I'm not sure it is what has been discussed. @kadel WDYT?

This is just a label. Yes, it is OpenShift specific, but there is no harm in always applying this. It might be even better, as there might be another tooling that could eventually leverage that.
And the only OpenShift specific is it's name, functionality wise there is nothing that should tie this only to openshift

rm3l · 2022-09-13T09:16:02Z

pkg/component/component.go

+	if metadata.Language != "" {
+		return metadata.Language
+	}
+	return metadata.ProjectType


I know we already discussed about using the language first and then the project type, but after quickly testing on Dev Sandbox, I think we should use the project type first.

The Deployment I created manually in Dev Sandbox has the following label: app.openshift.io/runtime=nodejs, but the one created by odo has app.openshift.io/runtime=javascript, and the right icon is not showing up in Topology View.

What is the behaviour with Java projects?

With a Quarkus app, we have the right icon for the language (Java):

If I manually edit the label to use the project type (quarkus) defined in the Devfile, we have a more accurate icon for this:

Same point for a SpringBoot app: we currently have the same icon as Quarkus for the language (Java). But if I set the label to the project type (spring) defined in the Devfile, then we have a more specific icon for this component:

This is why I was thinking about using the project type first instead.

I guess we never will be able to reliably cover all the cases. If we use projectType then there will be a problem with java-maven. I guess we just need to check what approach works for most of the cases.

I'm ok if we go with projectTyp

sonarcloud · 2022-09-13T13:56:27Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

rm3l · 2022-09-13T14:45:05Z

Thanks for the new changes.

/approve

openshift-ci · 2022-09-13T14:45:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rm3l]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cdrage · 2022-09-13T18:21:04Z

Tested and works well 👍

Thanks @feloy

/lgtm

feloy · 2022-09-14T06:55:29Z

Flaky test

odo add binding interactive command tests
/go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:16
  when the component is bootstrapped
  /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:48
    when binding to a service in a different namespace
    /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:219
      [It] should error out if service is not found in the namespace selected
      /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:240

/override ci/prow/v4.10-integration-e2e

openshift-ci · 2022-09-14T06:55:36Z

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.10-integration-e2e

In response to this:

Flaky test

odo add binding interactive command tests
/go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:16
 when the component is bootstrapped
 /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:48
   when binding to a service in a different namespace
   /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:219
     [It] should error out if service is not found in the namespace selected
     /go/src/github.com/redhat-developer/odo/tests/integration/interactive_add_binding_test.go:240

/override ci/prow/v4.10-integration-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

…loper#6112) * Add runtime label based on metadata:language/projectType * Add integration tests for odo dev * odo deploy integration test * Reverse priority between pojectType and language

openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Sep 12, 2022

openshift-ci bot requested review from cdrage and rm3l September 12, 2022 07:55

valaparthvi reviewed Sep 12, 2022

View reviewed changes

feloy force-pushed the feature-6044/runtime-label branch 3 times, most recently from e046590 to a9de64d Compare September 12, 2022 13:27

rm3l reviewed Sep 13, 2022

View reviewed changes

feloy added 4 commits September 13, 2022 15:51

Add runtime label based on metadata:language/projectType

1aa5c67

Add integration tests for odo dev

17df9db

odo deploy integration test

4be7b6e

Reverse priority between pojectType and language

6e50b8f

feloy force-pushed the feature-6044/runtime-label branch from a9de64d to 6e50b8f Compare September 13, 2022 13:55

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Sep 13, 2022

openshift-ci bot assigned cdrage Sep 13, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Sep 13, 2022

openshift-merge-robot merged commit 32303b1 into redhat-developer:main Sep 14, 2022

rm3l mentioned this pull request Sep 14, 2022

add app.openshift.io/runtime label to resources created by odo #6044

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add runtime label based on metadata:language/projectType #6112

Add runtime label based on metadata:language/projectType #6112

feloy commented Sep 12, 2022 •

edited

netlify bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

valaparthvi Sep 12, 2022 •

edited

feloy Sep 12, 2022

kadel Sep 13, 2022

rm3l Sep 13, 2022

feloy Sep 13, 2022 •

edited

rm3l Sep 13, 2022

rm3l Sep 13, 2022

kadel Sep 13, 2022 •

edited

sonarcloud bot commented Sep 13, 2022

rm3l commented Sep 13, 2022

openshift-ci bot commented Sep 13, 2022

cdrage commented Sep 13, 2022

feloy commented Sep 14, 2022

openshift-ci bot commented Sep 14, 2022

Add runtime label based on metadata:language/projectType #6112

Add runtime label based on metadata:language/projectType #6112

Conversation

feloy commented Sep 12, 2022 • edited

netlify bot commented Sep 12, 2022 • edited

✅ Deploy Preview for odo-docusaurus-preview canceled.

odo-robot bot commented Sep 12, 2022 • edited

odo-robot bot commented Sep 12, 2022 • edited

odo-robot bot commented Sep 12, 2022 • edited

odo-robot bot commented Sep 12, 2022 • edited

odo-robot bot commented Sep 12, 2022 • edited

valaparthvi Sep 12, 2022 • edited

Choose a reason for hiding this comment

feloy Sep 12, 2022

Choose a reason for hiding this comment

kadel Sep 13, 2022

Choose a reason for hiding this comment

rm3l Sep 13, 2022

Choose a reason for hiding this comment

feloy Sep 13, 2022 • edited

Choose a reason for hiding this comment

rm3l Sep 13, 2022

Choose a reason for hiding this comment

rm3l Sep 13, 2022

Choose a reason for hiding this comment

kadel Sep 13, 2022 • edited

Choose a reason for hiding this comment

sonarcloud bot commented Sep 13, 2022

rm3l commented Sep 13, 2022

openshift-ci bot commented Sep 13, 2022

cdrage commented Sep 13, 2022

feloy commented Sep 14, 2022

openshift-ci bot commented Sep 14, 2022

feloy commented Sep 12, 2022 •

edited

netlify bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

odo-robot bot commented Sep 12, 2022 •

edited

valaparthvi Sep 12, 2022 •

edited

feloy Sep 13, 2022 •

edited

kadel Sep 13, 2022 •

edited