This repository has been archived by the owner on Jul 25, 2022. It is now read-only.
Do not require chronyd to be installed on storage servers #2307
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
When installing a minimum build of centos it will not include chronyd in the installation. This currently prevents users from being able to add storage servers. Update the disable chronyd step to be smarter such that the command does not error if it is not installed. Signed-off-by: johnsonw <[email protected]>
|
This show the failure to stop the chronyd service if it is not installed: And this shows that after the patch is applied, the exception is swallowed and the process will continue: |
ip1981
approved these changes
Oct 6, 2020
| except AgentException as e: | ||
| t, v, tb = sys.exc_info() | ||
|
|
||
| if "Unknown busctl" in str(e): |
There was a problem hiding this comment.
Is there a more clear way to say no such unit? 🙂
There was a problem hiding this comment.
Yeah, I agree this is frustrating and ugly. If you know of a better way to check please let me know.
There was a problem hiding this comment.
Instead of trying to stop and handling the error. We should just check the state first and then stop iff it's already installed / running
mkpankov
reviewed
Oct 7, 2020
| if "Unknown busctl" in str(e): | ||
| return "" | ||
| else: | ||
| raise t, v, tb |
There was a problem hiding this comment.
Why explicitly raise with values? I thought just raise would re-raise last exception.
jgrund
suggested changes
Oct 20, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
When installing a minimum build of centos it will not include chronyd in
the installation. This currently prevents users from being able to add
storage servers. Update the disable chronyd step to be smarter such that
the command does not error if it is not installed.
Signed-off-by: johnsonw [email protected]
This change is