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.

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

mgr/cephadm: validating tuned profile specification #47786

Merged
merged 1 commit into from Sep 13, 2022
Merged

mgr/cephadm: validating tuned profile specification #47786

merged 1 commit into from Sep 13, 2022

Conversation

rkachach
Copy link
Contributor

@rkachach rkachach commented Aug 24, 2022 •
edited

Following checks are add as part of this PR:

  1. cephadm returns an error in case invalid tunable is mentioned under “settings” in YAML spec.
  2. cephadm gives an error if “settings” in YAML spec is empty.
  3. cephadm gives error if the specified placement is invalid (could not be honored).

fixes: https://tracker.ceph.com/issues/57192
fixes: https://tracker.ceph.com/issues/57192

Signed-off-by: Redouane Kachach rkachach@redhat.com

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@rkachach rkachach requested a review from a team as a code owner August 24, 2022 12:00
@rkachach rkachach requested a review from adk3798 August 26, 2022 07:38
adk3798
adk3798 reviewed Aug 29, 2022
View reviewed changes
src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved
src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved
src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved
src/pybind/mgr/cephadm/module.py Show resolved Hide resolved
src/pybind/mgr/cephadm/module.py Show resolved Hide resolved
@rkachach rkachach requested a review from adk3798 August 31, 2022 09:14
@rkachach
Copy link
Contributor Author

jenkins test make check

adk3798
adk3798 approved these changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@adk3798 adk3798 left a comment

Choose a reason for hiding this comment

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

LGTM, minor comments on error messages

src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved
src/pybind/mgr/cephadm/module.py Outdated Show resolved Hide resolved
@adk3798
Copy link
Contributor

adk3798 commented Aug 31, 2022

@rkachach also, seeing make check fail with

ceph/deployment/hostspec.py:135:16: E127 continuation line over-indented for visual indent
ceph/deployment/hostspec.py:136:16: E127 continuation line over-indented for visual indent
ceph/deployment/hostspec.py:137:16: E127 continuation line over-indented for visual indent

from tox-python-common. Not sure exactly why it's saying that as you aren't changing those exact lines, but the test doesn't seem to fail on main branch so I guess you'll have to address it here.

@rkachach
Copy link
Contributor Author

@rkachach also, seeing make check fail with

ceph/deployment/hostspec.py:135:16: E127 continuation line over-indented for visual indent
ceph/deployment/hostspec.py:136:16: E127 continuation line over-indented for visual indent
ceph/deployment/hostspec.py:137:16: E127 continuation line over-indented for visual indent

from tox-python-common. Not sure exactly why it's saying that as you aren't changing those exact lines, but the test doesn't seem to fail on main branch so I guess you'll have to address it here.

fixed in the last forced push 👍

@adk3798
Copy link
Contributor

adk3798 commented Sep 2, 2022

jenkins test dashboard cephadm

@rkachach
Copy link
Contributor Author

rkachach commented Sep 2, 2022

That last forced push is just to fix a typo + change in the text. Shouldn't affect the ongoing testing.

@rkachach rkachach changed the title mgr/cephadm: validating TunedProfileSpec placement mgr/cephadm: validating tuned profile specification Sep 6, 2022
@rkachach rkachach mentioned this pull request Sep 6, 2022
Closed
14 tasks
@rkachach
Copy link
Contributor Author

rkachach commented Sep 6, 2022

jenkins test make check

@rkachach
Copy link
Contributor Author

rkachach commented Sep 7, 2022

jenkins test make check

1 similar comment
@adk3798
Copy link
Contributor

adk3798 commented Sep 7, 2022

jenkins test make check

@adk3798
Copy link
Contributor

adk3798 commented Sep 13, 2022

https://pulpito.ceph.com/adking-2022-09-12_11:00:28-orch:cephadm-wip-adk-testing-2022-09-11-1805-distro-default-smithi/

failures tracked by https://tracker.ceph.com/issues/57287 and unrelated to any PRs in this run

@adk3798 adk3798 merged commit 581098e into ceph:main Sep 13, 2022
@adk3798 adk3798 mentioned this pull request Sep 13, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adk3798 adk3798 adk3798 approved these changes

Assignees
No one assigned
Labels
cephadm pybind wip-adk-testing
Projects
None yet
Milestone
No milestone
2 participants
@rkachach @adk3798