Skip to content

mon, doc: overriding ec profile requires --yes-i-really-mean-it#56287

Merged
zdover23 merged 1 commit intoceph:mainfrom
rzarzynski:wip-ec-profile-set-paranoid-on-override
Mar 24, 2024
Merged

mon, doc: overriding ec profile requires --yes-i-really-mean-it#56287
zdover23 merged 1 commit intoceph:mainfrom
rzarzynski:wip-ec-profile-set-paranoid-on-override

Conversation

@rzarzynski
Copy link
Contributor

@rzarzynski rzarzynski commented Mar 19, 2024

This is per https://tracker.ceph.com/issues/64333#note-17 describing driving factors of a catastrophic cluster failure.

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

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
  • jenkins test rook e2e

:Description: Override an existing profile by the same name, and allow
setting a non-4K-aligned stripe_unit.
setting a non-4K-aligned stripe_unit. The former can be
dangerous, and thus ``--yes-i-really-mean-it`` must be
Copy link
Contributor

Choose a reason for hiding this comment

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

s/must be/must be used/
or
s/must be/must be invoked/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest s/The former/This/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention was call dangerous just the overriding of existing, not setting an exotic stripe size.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest /The former/Overriding an existing profile/

@rzarzynski rzarzynski force-pushed the wip-ec-profile-set-paranoid-on-override branch from 2c2aae5 to d967b49 Compare March 19, 2024 13:53
This is per https://tracker.ceph.com/issues/64333#note-17 describing
driving factors of a catastrophic cluster failure.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
@rzarzynski rzarzynski force-pushed the wip-ec-profile-set-paranoid-on-override branch from d967b49 to 629ba7b Compare March 19, 2024 14:13
@zdover23
Copy link
Contributor

jenkins test api

Copy link
Member

@jdurgin jdurgin left a comment

Choose a reason for hiding this comment

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

talking to radek, we did see a couple niche valid uses of changing a ec profile - switching from one plugin to another with the same encoding scheme (e.g. jerasure to isa for reed solomon) and possibly other parameter updates that don't affect on-disk format for other plugins. So I'm ok with keeping the command, and requiring extra confirmation.

@zdover23 zdover23 merged commit 063fb89 into ceph:main Mar 24, 2024
@zdover23
Copy link
Contributor

#56434 - Squid backport
#56435 - Reef backport
Quincy backport will be done if Radek confirms that the code changes made here apply to the Quincy release branch.

@ljflores
Copy link
Member

Looks like this PR was merged a little early- there is some fallout in teuthology. @rzarzynski FYI

See https://tracker.ceph.com/issues/65183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants