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

Change ephemeral default to false #5795

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

kadel
Copy link
Member

@kadel kadel commented Jun 6, 2022

What type of PR is this:

/kind feature

Feel free to use other labels as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->

What does this PR do / why we need it:

Which issue(s) this PR fixes:
fixes #5241

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@netlify
Copy link

netlify bot commented Jun 6, 2022

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit ba15cda
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/62b5b2f9c3c2af000854646a
😎 Deploy Preview https://deploy-preview-5795--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@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 Jun 6, 2022
@kadel kadel changed the title Change ephemeral default to false [WIP] Change ephemeral default to false Jun 6, 2022
@openshift-ci openshift-ci bot requested review from dharmit and valaparthvi June 6, 2022 14:28
@openshift-ci 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 Jun 6, 2022
@odo-robot
Copy link

odo-robot bot commented Jun 6, 2022

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

@odo-robot
Copy link

odo-robot bot commented Jun 6, 2022

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

@odo-robot
Copy link

odo-robot bot commented Jun 6, 2022

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

@odo-robot
Copy link

odo-robot bot commented Jun 6, 2022

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

@odo-robot
Copy link

odo-robot bot commented Jun 6, 2022

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

@kadel kadel force-pushed the ephemeral branch 2 times, most recently from e1a49ba to 0e32501 Compare June 7, 2022 12:58
@feloy
Copy link
Contributor

feloy commented Jun 7, 2022

All the integration tests are now running with non-ephemeral volumes for component's project directory.

IMO, this has some effect on the limits of resources on the cluster, and could slow down the tests (with 32 tests running in parallel, we can easily find Pod in Pending state for lack of memory)

Perhaps do we want to switch the preference to ephemeral during the tests?

@kadel kadel changed the title [WIP] Change ephemeral default to false Change ephemeral default to false Jun 7, 2022
@openshift-ci openshift-ci bot removed 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 Jun 7, 2022
@feloy
Copy link
Contributor

feloy commented Jun 13, 2022

All the integration tests are now running with non-ephemeral volumes for component's project directory.

IMO, this has some effect on the limits of resources on the cluster, and could slow down the tests (with 32 tests running in parallel, we can easily find Pod in Pending state for lack of memory)

Perhaps do we want to switch the preference to ephemeral during the tests?

@kadel, what's your opinion?

@kadel
Copy link
Member Author

kadel commented Jun 15, 2022

All the integration tests are now running with non-ephemeral volumes for component's project directory.

IMO, this has some effect on the limits of resources on the cluster, and could slow down the tests (with 32 tests running in parallel, we can easily find Pod in Pending state for lack of memory)

Perhaps do we want to switch the preference to ephemeral during the tests?

This is a good point.
I'll change our tests to use ephemeral by default.
There already is at least one test that verifies that ephemeral=false works correctly (https://github.com/redhat-developer/odo/blob/main/tests/integration/devfile/cmd_dev_test.go#L269)

@feloy
Copy link
Contributor

feloy commented Jun 16, 2022

There already is at least one test that verifies that ephemeral=false works correctly (https://github.com/redhat-developer/odo/blob/main/tests/integration/devfile/cmd_dev_test.go#L269)

You will need to update this test to now force the odo set preference command, or it will refuse because the value is now already set:

[odo] ? ephemeral is already set. Do you want to override it in the config (y/N)
I0616 07:53:42.259922   22593 common.go:17] Encountered an error processing prompt: EOF
[odo] Aborted by the user

@feloy
Copy link
Contributor

feloy commented Jun 20, 2022

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jun 20, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: feloy

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jun 20, 2022
@kadel kadel changed the title Change ephemeral default to false [WIP] Change ephemeral default to false Jun 20, 2022
@openshift-ci 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 Jun 20, 2022
Signed-off-by: Tomas Kral <tkral@redhat.com>
@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.0% 0.0% Duplication

@feloy
Copy link
Contributor

feloy commented Jun 24, 2022

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jun 24, 2022
@kadel kadel changed the title [WIP] Change ephemeral default to false Change ephemeral default to false Jun 24, 2022
@openshift-ci openshift-ci bot removed 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 Jun 24, 2022
@feloy
Copy link
Contributor

feloy commented Jun 24, 2022

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

Tests pass on IBM Cloud

@openshift-ci
Copy link

openshift-ci bot commented Jun 24, 2022

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

In response to this:

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

Tests pass on IBM Cloud

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 Jun 24, 2022

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

@openshift-ci
Copy link

openshift-ci bot commented Jun 24, 2022

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

In response to this:

/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.

@openshift-ci openshift-ci bot merged commit d72b30c into redhat-developer:main Jun 24, 2022
cdrage pushed a commit to cdrage/odo that referenced this pull request Aug 31, 2022
Signed-off-by: Tomas Kral <tkral@redhat.com>
rm3l added a commit to rm3l/odo that referenced this pull request Sep 7, 2022
Since redhat-developer#5795, the default value is now False,
but the documentation was outdated.
openshift-merge-robot pushed a commit that referenced this pull request Sep 7, 2022
Since #5795, the default value is now False,
but the documentation was outdated.
@dharmit dharmit mentioned this pull request Sep 9, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. 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.

Set Ephemeral to false by default in odo preferences
2 participants