Support `autoBuild` and `deployByDefault` on Image and Kubernetes/OpenShift components #6654

rm3l · 2023-03-10T14:43:35Z

What type of PR is this:
/kind feature

What does this PR do / why we need it:
This adds support for the following fields:

autoBuild on Image Components
deployByDefault on Kubernetes and OpenShift Components

Which issue(s) this PR fixes:
Fixes #5694

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:
Documentation is here: https://deploy-preview-6654--odo-docusaurus-preview.netlify.app/docs/development/devfile#how-odo-determines-components-that-are-applied-automatically

netlify · 2023-03-10T14:43:40Z

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	`dc0fe61`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/642c4be1b8e80b000842648c

openshift-ci · 2023-03-10T14:43:41Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

odo-robot · 2023-03-10T14:49:49Z

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

odo-robot · 2023-03-10T14:49:59Z

NoCluster Tests on commit d344125 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-10T14:51:34Z

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

odo-robot · 2023-03-10T15:09:05Z

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

odo-robot · 2023-03-10T15:26:40Z

Windows Tests (OCP) on commit d344125 finished with errors.
View logs: TXT HTML

odo-robot · 2023-03-10T15:36:24Z

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

odo-robot · 2023-03-10T15:57:09Z

OpenShift Tests on commit d344125 finished with errors.
View logs: TXT HTML

odo-robot · 2023-03-21T09:23:38Z

Kubernetes Docs Tests on commit 8876e1a finished successfully.
View logs: TXT HTML

It will be used for integration tests.

This will allow supporting cases where we need to run a custom command, but this is not implemented yet on odo (cases with 'odo dev --debug' and 'odo deploy'). In this case, this helper will allow updating the Devfile for example to unmark the current default command as non-default, and mark the custom one as default.

'odo build-images' should build all images regardless of the 'autoBuild' property.

This helps understand what resources are being patched.

…t of image components to auto-create See devfile/api#852 (comment) for more context.

valaparthvi · 2023-04-04T08:46:46Z

tests/examples/source/devfiles/nodejs/devfile-autobuild-deploybydefault.yaml

+
+  - apply:
+      component: autobuild-true-and-referenced
+    id: image-autobuild-true-and-referenced


nit:
I think command ID could simply be image-autobuild-true because if a command is defined for a component, it most definitely is referenced.

Same for other commands.

Done in 65812b8 (#6720)

valaparthvi · 2023-04-04T08:59:17Z

tests/integration/cmd_dev_debug_test.go

+					helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
+					helper.CopyExample(filepath.Join("source", "nodejs", "Dockerfile"), filepath.Join(commonVar.Context, "Dockerfile"))


Can you not use source/nodejs directly?

Suggested change

helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)

helper.CopyExample(filepath.Join("source", "nodejs", "Dockerfile"), filepath.Join(commonVar.Context, "Dockerfile"))

helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)

The source/nodejs project does not have a debug command in its sources, that's why. I remember that's why I did not use it. But you're right - I'll add the debug command and use it instead.

Done in be23bbf (#6720)

valaparthvi · 2023-04-04T09:02:21Z

tests/integration/cmd_dev_debug_test.go

+						var err error
+						var envvars []string
+						if podman {
+							envvars = append(envvars, "ODO_PUSH_IMAGES=false")


Side note: I wonder if we should implement ODO_PUSH_IMAGES for odo dev on cluster as well.

On cluster, we are using PODMAN_CMD=echo which is faster IMO. But yes, we could do the same thing on cluster.

valaparthvi · 2023-04-04T09:07:41Z

tests/integration/cmd_dev_debug_test.go

+						if podman {
+							devSession.WaitEnd()
+						}


Why only for podman? I assume this has something to do with namespaces? Every test on cluster uses a different namespace, which is not true for podman. Still, I wonder if for the sake of simplicity we should wait for command to end on cluster as well.

Or, if you think this would save us time, perhaps we can implement this for all the remaining tests, in a separate PR of course.

Yes, that was the idea, to save some time. On the cluster, it is not needed to wait, as we'll be cleaning the entire namespace after the test. On Podman, I had some issues if not waiting, due to how Podman kind of isolates things.
We can take a look in a different PR, if we can save more time on other tests.

valaparthvi · 2023-04-04T09:12:36Z

tests/integration/cmd_dev_debug_test.go

+								linesErr, _ := helper.ExtractLines(stderr)
+								var skipped []string
+								for _, l := range linesErr {
+									if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {


Just so that it is easier to understand, would you mind adding a sample line in the comments?

Suggested change

if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {

// Example line: Kubernetes components are not supported on Podman. Skipping: k8s-deploybydefault-true-and-referenced, k8s-deploybydefault-true-and-not-referenced.

if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {

See a85e969 (#6720)

valaparthvi · 2023-04-04T11:07:32Z

pkg/libdevfile/component_kubernetes.go

+		if comp.Kubernetes == nil && comp.Openshift == nil {
+			continue
+		}


Same comment as above, I don't think this condition will ever be hit since we are filtering the components based on their dedicated types.

Done in 9544a55 (#6720)

pkg/watch/status.go

pkg/devfile/adapters/kubernetes/component/push.go

valaparthvi · 2023-04-04T11:21:57Z

/hold cancel

valaparthvi · 2023-04-04T11:24:33Z

@rm3l The changes I've suggested are more of nit picks. Since Philippe has already approved the PR, if we want to get this PR in with this release, I am fine if you want to take them up in a followup PR.

Co-authored-by: Parthvi Vala <pvala@redhat.com>

rm3l · 2023-04-04T13:57:20Z

@rm3l The changes I've suggested are more of nit picks. Since Philippe has already approved the PR, if we want to get this PR in with this release, I am fine if you want to take them up in a followup PR.

Actually, there were some comments that made sense :) So I applied them. I'll make the other changes in a subsequent PR. Thanks for the feedback!

rm3l · 2023-04-04T15:10:43Z


Summarizing 2 Failures:
  [FAIL] odo logs command tests when component is created and odo logs is executed when running in both Dev and Deploy mode [It] should successfully show logs of the running component
  /go/odo_1/tests/helper/helper_generic.go:79
  [FAIL] odo logs command tests when component is created and odo logs is executed when running in Dev mode [It] should successfully show logs of the running component
  /go/odo_1/tests/helper/helper_generic.go:79

Ran 439 of 800 Specs in 1251.443 seconds
FAIL! -- 437 Passed | 2 Failed | 0 Pending | 361 Skipped


Ginkgo ran 1 suite in 21m57.680223467s

Flaky odo logs tests (#6619)

/override OpenShift-Integration-tests/OpenShift-Integration-tests

openshift-ci · 2023-04-04T15:10:48Z

@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:


Summarizing 2 Failures:
 [FAIL] odo logs command tests when component is created and odo logs is executed when running in both Dev and Deploy mode [It] should successfully show logs of the running component
 /go/odo_1/tests/helper/helper_generic.go:79
 [FAIL] odo logs command tests when component is created and odo logs is executed when running in Dev mode [It] should successfully show logs of the running component
 /go/odo_1/tests/helper/helper_generic.go:79

Ran 439 of 800 Specs in 1251.443 seconds
FAIL! -- 437 Passed | 2 Failed | 0 Pending | 361 Skipped


Ginkgo ran 1 suite in 21m57.680223467s

Flaky odo logs tests (#6619)

/override OpenShift-Integration-tests/OpenShift-Integration-tests

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.

valaparthvi · 2023-04-04T15:31:02Z

/lgtm

rm3l · 2023-04-04T15:43:48Z

/hold

I noticed that the odo logs tests were consistently failing and might not actually be flaky. I'm investigating why..

Set deployByDefault to false on innerloop-pod component. Otherwise, since it is not referenced by any apply command, it will be automatically created by *both* 'odo dev' and 'odo deploy'.

sonarcloud · 2023-04-04T16:10:50Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
0.0% Duplication

rm3l · 2023-04-04T16:15:04Z

/hold cancel

The issues with odo logs should be fixed (well, at least, we should be back to the flaky state we see with #6619). I noticed that there were still a few places where boolean fields are being set to their default values by the Devfile parser (like in odo init after downloading the Devfile and parsing it), which caused some issues because autoBuild and deployByDefault were set to false.

feloy · 2023-04-04T16:44:24Z

/lgtm

rm3l · 2023-04-05T05:25:10Z

  [FAILED] Timed out after 180.000s.
  Expected process to exit.  It did not.
  In [AfterEach] at: /go/odo_1/tests/helper/helper_dev.go:200 @ 04/04/23 16:46:29.489
------------------------------
SSSâ€¢SSSSSSâ€¢â€¢â€¢Sâ€¢â€¢â€¢â€¢â€¢SSSâ€¢â€¢â€¢SSâ€¢Sâ€¢Sâ€¢â€¢Sâ€¢Sâ€¢â€¢â€¢Sâ€¢â€¢Sâ€¢â€¢SSâ€¢Sâ€¢Sâ€¢â€¢â€¢â€¢SSSâ€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢

Summarizing 1 Failure:
  [FAIL] odo dev command tests when Starting a PostgreSQL service when creating local files and dir and running odo dev - without metadata.name [AfterEach] when deleting local files and dir and waiting for sync should not list deleted dir and file in container
  /go/odo_1/tests/helper/helper_dev.go:200

Ran 439 of 800 Specs in 1531.138 seconds
FAIL! -- 438 Passed | 1 Failed | 0 Pending | 361 Skipped


Ginkgo ran 1 suite in 26m34.866445083s

Flaky test

/override OpenShift-Integration-tests/OpenShift-Integration-tests

openshift-ci · 2023-04-05T05:25:14Z

@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Integration-tests

In response to this:

 [FAILED] Timed out after 180.000s.
 Expected process to exit.  It did not.
 In [AfterEach] at: /go/odo_1/tests/helper/helper_dev.go:200 @ 04/04/23 16:46:29.489
------------------------------
SSSâ€¢SSSSSSâ€¢â€¢â€¢Sâ€¢â€¢â€¢â€¢â€¢SSSâ€¢â€¢â€¢SSâ€¢Sâ€¢Sâ€¢â€¢Sâ€¢Sâ€¢â€¢â€¢Sâ€¢â€¢Sâ€¢â€¢SSâ€¢Sâ€¢Sâ€¢â€¢â€¢â€¢SSSâ€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢â€¢

Summarizing 1 Failure:
 [FAIL] odo dev command tests when Starting a PostgreSQL service when creating local files and dir and running odo dev - without metadata.name [AfterEach] when deleting local files and dir and waiting for sync should not list deleted dir and file in container
 /go/odo_1/tests/helper/helper_dev.go:200

Ran 439 of 800 Specs in 1531.138 seconds
FAIL! -- 438 Passed | 1 Failed | 0 Pending | 361 Skipped


Ginkgo ran 1 suite in 26m34.866445083s

Flaky test

/override OpenShift-Integration-tests/OpenShift-Integration-tests

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.

rm3l · 2023-04-05T05:26:05Z


  [FAILED] Expected
      <*url.Error | 0xc000018060>: {
          Op: "Post",
          URL: "http://127.0.0.1:57428/api/newuser",
          Err: <*errors.errorString | 0xc00008a130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3615/tests/e2escenarios/e2e_test.go:306 @ 04/04/23 11:34:48.306

  There were additional failures detected.  To view them in detail run ginkgo -vv
------------------------------

Summarizing 1 Failure:
  [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
  C:/Users/Administrator.ANSIBLE-TEST-VS/3615/tests/e2escenarios/e2e_test.go:306

Ran 7 of 7 Specs in 206.889 seconds
FAIL! -- 6 Passed | 1 Failed | 0 Pending | 0 Skipped

Flaky E2E test (#6582)

/override windows-integration-test/Windows-test

openshift-ci · 2023-04-05T05:26:10Z

@rm3l: Overrode contexts on behalf of rm3l: windows-integration-test/Windows-test

In response to this:


 [FAILED] Expected
     <*url.Error | 0xc000018060>: {
         Op: "Post",
         URL: "http://127.0.0.1:57428/api/newuser",
         Err: <*errors.errorString | 0xc00008a130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3615/tests/e2escenarios/e2e_test.go:306 @ 04/04/23 11:34:48.306

 There were additional failures detected.  To view them in detail run ginkgo -vv
------------------------------

Summarizing 1 Failure:
 [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
 C:/Users/Administrator.ANSIBLE-TEST-VS/3615/tests/e2escenarios/e2e_test.go:306

Ran 7 of 7 Specs in 206.889 seconds
FAIL! -- 6 Passed | 1 Failed | 0 Pending | 0 Skipped

Flaky E2E test (#6582)

/override windows-integration-test/Windows-test

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.

…Default`) (#6720) * Remove additional non-nil checks on component fields since they are already filtered by type We are confident that the fields we expect (comp.{Image,Kubernetes,OpenShift}) are non-nil. * Rename 'libdevfile.GetImageComponentsToPush' into 'libdevfile.GetImageComponentsToPushAutomatically' * Document with examples cases where 'deployByDefault' and 'autoBuild' are `false` and the component is not referenced * Rename 'image-autobuild-true-and-referenced' command into 'image-autobuild-true' in test Devfile * Simplify parsing the output on Podman in the tests when looking for K8s/OpenShift components * Use 'source/nodejs' instead of 'source/devfiles/nodejs/project' in the tests * Revert "Rename 'image-autobuild-true-and-referenced' command into 'image-autobuild-true' in test Devfile" This reverts commit 65812b8.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 10, 2023

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

rm3l added the area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. label Mar 10, 2023

rm3l mentioned this pull request Mar 10, 2023

implement autoBuild and deployByDefault #5694

Closed

rm3l force-pushed the 5694-implement-autobuild-and-deploybydefault branch from ec42a2c to 055f71a Compare March 13, 2023 09:06

rm3l force-pushed the 5694-implement-autobuild-and-deploybydefault branch from 055f71a to 683389a Compare March 21, 2023 08:24

rm3l force-pushed the 5694-implement-autobuild-and-deploybydefault branch from 8d11bed to 3cfabfe Compare March 29, 2023 08:15

rm3l closed this Mar 29, 2023

rm3l reopened this Mar 29, 2023

rm3l closed this Mar 29, 2023

rm3l reopened this Mar 29, 2023

rm3l force-pushed the 5694-implement-autobuild-and-deploybydefault branch from eaf492f to 04052e5 Compare March 31, 2023 16:33

rm3l added 8 commits April 4, 2023 09:54

Add sample Devfile with multiple autoBuild/deployByDefault combinations

dddb2dc

It will be used for integration tests.

Add test cases for 'odo dev'

35a6e50

Add test cases for 'odo deploy'

125ff54

Add test cases for 'odo build-images'

510c5db

'odo build-images' should build all images regardless of the 'autoBuild' property.

Display the spinner when creating or updating Kubernetes resources

344879a

This helps understand what resources are being patched.

Handle deployByDefault on K8s and OpenShift components

cbbd52a

Add 'devfile.GetImageComponentsToPush' functions that returns the lis…

399b42c

…t of image components to auto-create See devfile/api#852 (comment) for more context.

valaparthvi suggested changes Apr 4, 2023

View reviewed changes

openshift-ci bot assigned valaparthvi Apr 4, 2023

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 4, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Apr 4, 2023

Apply suggestions from code review

7a1f1a0

Co-authored-by: Parthvi Vala <pvala@redhat.com>

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 4, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Apr 4, 2023

rm3l added 2 commits April 4, 2023 17:46

fixup! Do not set default values when parsing a Devfile

3673d01

Fix devfile-deploy-functional-pods.yaml (used in 'odo logs' tests)

dc0fe61

Set deployByDefault to false on innerloop-pod component. Otherwise, since it is not referenced by any apply command, it will be automatically created by *both* 'odo dev' and 'odo deploy'.

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 4, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Apr 4, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 4, 2023

openshift-merge-robot merged commit cc2e8de into redhat-developer:main Apr 5, 2023

rm3l deleted the 5694-implement-autobuild-and-deploybydefault branch April 5, 2023 06:55

rm3l mentioned this pull request Apr 5, 2023

Follow-up to comments on #6654 (support for autoBuild and deployByDefault) #6720

Merged

3 tasks

		helper.CopyExample(filepath.Join("source", "devfiles", "nodejs", "project"), commonVar.Context)
		helper.CopyExample(filepath.Join("source", "nodejs", "Dockerfile"), filepath.Join(commonVar.Context, "Dockerfile"))

	if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {
	// Example line: Kubernetes components are not supported on Podman. Skipping: k8s-deploybydefault-true-and-referenced, k8s-deploybydefault-true-and-not-referenced.
	if strings.Contains(l, "Kubernetes components are not supported on Podman. Skipping:") {

Support autoBuild and deployByDefault on Image and Kubernetes/OpenShift components #6654

Support autoBuild and deployByDefault on Image and Kubernetes/OpenShift components #6654

Conversation

rm3l commented Mar 10, 2023 • edited Loading

netlify bot commented Mar 10, 2023 • edited Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

openshift-ci bot commented Mar 10, 2023

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 10, 2023 • edited Loading

odo-robot bot commented Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valaparthvi commented Apr 4, 2023

valaparthvi commented Apr 4, 2023

rm3l commented Apr 4, 2023

rm3l commented Apr 4, 2023

openshift-ci bot commented Apr 4, 2023

valaparthvi commented Apr 4, 2023

rm3l commented Apr 4, 2023 • edited Loading

sonarcloud bot commented Apr 4, 2023

rm3l commented Apr 4, 2023 • edited Loading

feloy commented Apr 4, 2023

rm3l commented Apr 5, 2023

openshift-ci bot commented Apr 5, 2023

rm3l commented Apr 5, 2023

openshift-ci bot commented Apr 5, 2023

Support `autoBuild` and `deployByDefault` on Image and Kubernetes/OpenShift components #6654

Support `autoBuild` and `deployByDefault` on Image and Kubernetes/OpenShift components #6654

rm3l commented Mar 10, 2023 •

edited

Loading

netlify bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 10, 2023 •

edited

Loading

odo-robot bot commented Mar 21, 2023 •

edited

Loading

rm3l commented Apr 4, 2023 •

edited

Loading

rm3l commented Apr 4, 2023 •

edited

Loading