Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update devfile/library to support pod-overrides and container-overrides attributes and add integration test for it #6512

Conversation

valaparthvi
Copy link
Contributor

@valaparthvi valaparthvi commented Jan 18, 2023

What type of PR is this:
/kind feature

What does this PR do / why we need it:
This PR updates the devfile/library to devfile/library@0d04f79 to support pod-overrides and container-overrides. It also adds integration test for the same.

Which issue(s) this PR fixes:

Fixes #5977

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:
Steps to update the library:

  1. Devfile Update: go get github.com/devfile/library/v2@0d04f79
  2. Replace all the imports of github.com/devfile/library to github.com/devfile/library/v2
  3. go mod vendor && go mod tidy

@netlify
Copy link

netlify bot commented Jan 18, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 373572d
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63ce7bbe297ac70008bf1bbc

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Jan 18, 2023
@openshift-ci openshift-ci bot requested review from feloy and kadel January 18, 2023 17:48
@valaparthvi valaparthvi requested a review from rm3l January 18, 2023 17:49
@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@odo-robot
Copy link

odo-robot bot commented Jan 18, 2023

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

@valaparthvi valaparthvi reopened this Jan 19, 2023
@valaparthvi
Copy link
Contributor Author

/retest-required

)
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile-pod-container-overrides.yaml"), filepath.Join(commonVar.Context, "devfile.yaml"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use random name, as in #6504

@valaparthvi valaparthvi force-pushed the 5977_update_devfile_library branch 2 times, most recently from 0d80fd8 to 5d988b1 Compare January 20, 2023 08:36
helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
helper.CopyExampleDevFile(filepath.Join("source", "devfiles", "nodejs", "devfile-pod-container-overrides.yaml"), filepath.Join(commonVar.Context, "devfile.yaml"), helper.DevfileMetadataNameSetter(cmpName))
})
It("should override the content in the pod it creates for the component on the cluster", func() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: can we use the same test on Podman as well? Or if it does not work, maybe we can try to override some specific fields recognized by Podman..

@valaparthvi valaparthvi requested review from rm3l and feloy January 20, 2023 14:31
@feloy
Copy link
Contributor

feloy commented Jan 20, 2023

You get this error in podman tests with Podman v3 (in CI):

error starting container 78000590aeaab78ab0d0ed6f00de89bef493feb5900ba9d9f810c893f5822a6c: the requested cgroup controller `cpu` is not available: OCI runtime error

@valaparthvi
Copy link
Contributor Author

You get this error in podman tests with Podman v3 (in CI):

error starting container 78000590aeaab78ab0d0ed6f00de89bef493feb5900ba9d9f810c893f5822a6c: the requested cgroup controller `cpu` is not available: OCI runtime error

It passed on my local podman so I figured it would work here too.

@feloy
Copy link
Contributor

feloy commented Jan 20, 2023

You get this error in podman tests with Podman v3 (in CI):

error starting container 78000590aeaab78ab0d0ed6f00de89bef493feb5900ba9d9f810c893f5822a6c: the requested cgroup controller `cpu` is not available: OCI runtime error

It passed on my local podman so I figured it would work here too.

It is probably a restriction on the CI VM

@valaparthvi
Copy link
Contributor Author

You get this error in podman tests with Podman v3 (in CI):

error starting container 78000590aeaab78ab0d0ed6f00de89bef493feb5900ba9d9f810c893f5822a6c: the requested cgroup controller `cpu` is not available: OCI runtime error

It passed on my local podman so I figured it would work here too.

It is probably a restriction on the CI VM

I found this(https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error) on Podman FAQ, do you think we can set resource limit if that will not affect other tests?

@feloy
Copy link
Contributor

feloy commented Jan 23, 2023

You get this error in podman tests with Podman v3 (in CI):

error starting container 78000590aeaab78ab0d0ed6f00de89bef493feb5900ba9d9f810c893f5822a6c: the requested cgroup controller `cpu` is not available: OCI runtime error

It passed on my local podman so I figured it would work here too.

It is probably a restriction on the CI VM

I found this(https://github.com/containers/podman/blob/main/troubleshooting.md#26-running-containers-with-resource-limits-fails-with-a-permissions-error) on Podman FAQ, do you think we can set resource limit if that will not affect other tests?

I would modify another field for this test, as resources setting is too much error prone, and we don't specifically want to test modification of resources fields.

PS: I find this tweet very apropos ;) https://twitter.com/manekinekko/status/1617208921957040131?s=20&t=R8ofTwvwU_ypQTIjrlIBZQ

@valaparthvi valaparthvi reopened this Jan 23, 2023
Signed-off-by: Parthvi Vala <pvala@redhat.com>

Attempt at fixing CI failures

Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
Signed-off-by: Parthvi Vala <pvala@redhat.com>
@valaparthvi valaparthvi force-pushed the 5977_update_devfile_library branch from 5ce3198 to 373572d Compare January 23, 2023 12:21
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 23, 2023
@valaparthvi valaparthvi reopened this Jan 23, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.3% 0.3% Duplication

@rm3l
Copy link
Member

rm3l commented Jan 23, 2023

/override windows-integration-test/Windows-test

  Expected
      <*url.Error | 0xc0008a68d0>: {
          Op: "Post",
          URL: "http://127.0.0.1:60053/api/newuser",
          Err: <*errors.errorString | 0xc00007c130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3089/tests/e2escenarios/e2e_test.go:293

  There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
    [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3089/tests/helper/helper_filesystem.go:49
------------------------------


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/3089/tests/e2escenarios/e2e_test.go:293

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

Flaky E2E test.

@openshift-ci
Copy link

openshift-ci bot commented Jan 23, 2023

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

In response to this:

/override windows-integration-test/Windows-test

 Expected
     <*url.Error | 0xc0008a68d0>: {
         Op: "Post",
         URL: "http://127.0.0.1:60053/api/newuser",
         Err: <*errors.errorString | 0xc00007c130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3089/tests/e2escenarios/e2e_test.go:293

 There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
   [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3089/tests/helper/helper_filesystem.go:49
------------------------------


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/3089/tests/e2escenarios/e2e_test.go:293

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

Flaky E2E 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.

@feloy
Copy link
Contributor

feloy commented Jan 23, 2023

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

@openshift-ci
Copy link

openshift-ci bot commented Jan 23, 2023

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

In response to this:

/override ci/prow/v4.11-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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to specify ServiceAccount for odo dev
4 participants