-
Notifications
You must be signed in to change notification settings - Fork 55
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
Accept "adv" key as well as "thp" key #3700
Merged
mulkieran
merged 6 commits into
stratis-storage:master
from
mulkieran:issue_stratisd_3699
Oct 17, 2024
Merged
Accept "adv" key as well as "thp" key #3700
mulkieran
merged 6 commits into
stratis-storage:master
from
mulkieran:issue_stratisd_3699
Oct 17, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mulkieran
force-pushed
the
issue_stratisd_3699
branch
from
October 14, 2024 15:03
30751f7
to
94c2f0e
Compare
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
mulkieran
force-pushed
the
issue_stratisd_3699
branch
5 times, most recently
from
October 15, 2024 16:22
a2b02fb
to
2ea1cc6
Compare
@jbaublitz This is a port of #3701 . It has to distribute some bits of that PR into v2 as well, and that's probably the only thing worth checking for correctness. |
mulkieran
force-pushed
the
issue_stratisd_3699
branch
from
October 15, 2024 18:53
2ea1cc6
to
782e09d
Compare
rebased |
jbaublitz
approved these changes
Oct 16, 2024
The Clevis config specification allows both a "thp" key, which the code already checks for and an "adv" key, which the code has been unaware of. Change name of Clevis config verification method to match expanded specification. Change header documentation to match. Change error message to match other changes. Signed-off-by: mulhern <amulhern@redhat.com>
The "adv" value is either a string containing the filename or more JSON. Signed-off-by: mulhern <amulhern@redhat.com>
Do not strip config of thumbprint entry, as the comparison that required that operation has been removed. It would be nice to also check equality of URLs, if the pin is "tang". But that is a lot of work for a small benefit. If a user for some reason attempts to create a pool with the same name, the same devices, and the same pin, as the pool was created with previously, then stratisd will return a value indicating that the pool was already created even if something else about the Clevis config is different. This seems acceptable. Signed-off-by: mulhern <amulhern@redhat.com>
The return value was used to decide whether to remove a key/value pair, but that no longer needs to be removed. The argument passed to it was only used for a comparison which is no longer being made and an error message display that can use the value it was cloned from instead. If the config was malformed, then it would error early, i.e., before attempting to bind the first blockdev. Now it will just error when it tries to bind the first blockdev. This is simpler to reason about. Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
Signed-off-by: mulhern <amulhern@redhat.com>
mulkieran
force-pushed
the
issue_stratisd_3699
branch
from
October 16, 2024 20:16
782e09d
to
3335f2c
Compare
rebased. We expect all tests to succeed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #3699