Skip to content

fix container cpu resource conversion #18527

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.

Sign up for GitHub

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

Jump to bottom
Merged
sparkoo merged 2 commits into from
Dec 4, 2020
Merged

fix container cpu resource conversion #18527

sparkoo merged 2 commits into from
Dec 4, 2020

Conversation

@sparkoo
Copy link
Member

@sparkoo sparkoo commented Dec 3, 2020

Signed-off-by: Michal Vala mvala@redhat.com

What does this PR do?

Fix conversion of CPU resource into full cores for plugins.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

#18484

How to test this PR?

metadata:
  name: test-cpulimit22qho
attributes:
  persistVolumes: 'false'
components:
  - type: cheEditor
    reference: >-
      https://gist.githubusercontent.com/sparkoo/cb5105fa1f01f29bf73eb238f6ae622f/raw/090062b9eca6a25ef5226d566ef9626d8a2ca012/theia_meta.yaml
    alias: che-theia
apiVersion: 1.0.0

This devfile should now work and theia container should have

  cpuLimit: "1"
  cpuRequest: "0.03"

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

@sparkoo
fix container cpu resource conversion
07ac799 
Signed-off-by: Michal Vala <mvala@redhat.com>
@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Dec 3, 2020
@sparkoo
javadocs
68e412a 
Signed-off-by: Michal Vala <mvala@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Dec 3, 2020

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Dec 3, 2020

❌ E2E Happy path tests failed ❗

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

amisevsk
amisevsk approved these changes Dec 3, 2020
View reviewed changes
Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

LGTM

@skabashnyuk skabashnyuk added this to the 7.23 milestone Dec 4, 2020
@skabashnyuk skabashnyuk linked an issue Dec 4, 2020 that may be closed by this pull request
CPU request / limit in the plugin meta.yaml is not correctly translated on the k8s level #18484
Closed
1 task
@sparkoo
Copy link
Member Author

sparkoo commented Dec 4, 2020

[crw-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Dec 4, 2020

✅ E2E Happy path tests succeed 🎉

See Details

Tested with Eclipse Che Multiuser User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@sparkoo sparkoo merged commit 31dde19 into eclipse-che:master Dec 4, 2020
@sparkoo sparkoo deleted the gh18484-pluginCpuLimitRequestConversion branch December 4, 2020 12:08
@sparkoo sparkoo self-assigned this Dec 4, 2020
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Dec 4, 2020
@skabashnyuk skabashnyuk mentioned this pull request Dec 17, 2020
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@skabashnyuk skabashnyuk skabashnyuk approved these changes

@amisevsk amisevsk amisevsk approved these changes

@metlos metlos Awaiting requested review from metlos

@nickboldt nickboldt Awaiting requested review from nickboldt

@sleshchenko sleshchenko Awaiting requested review from sleshchenko

@mshaposhnik mshaposhnik Awaiting requested review from mshaposhnik

@ibuziuk ibuziuk Awaiting requested review from ibuziuk

Assignees

@sparkoo sparkoo

Labels

kind/bug Outline of a bug - must adhere to the bug report template.

Projects

None yet

Milestone

7.23

Development

Successfully merging this pull request may close these issues.

CPU request / limit in the plugin meta.yaml is not correctly translated on the k8s level

4 participants

@sparkoo @che-bot @skabashnyuk @amisevsk