hooks/1.0.0: Fix 'annotation' -> 'annotations' in JSON #887
Conversation
This file contains hidden or 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
The continue here is from 5676597 (hooks/read: Ignore IsNotExist for JSON files in ReadDir, 2018-04-27, containers#686), where it was intended to silently ignore missing JSON files. However, the old logic was also silently ignoring not-exist errors from the os.Stat(hook.Hook.Path) from 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, containers#686). This commit adjusts the check so JSON not-exist errors continue to be silently ignored while hook executable not-exist errors become fatal. Signed-off-by: W. Trevor King <wking@tremily.us>
This typo from 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, containers#686) was causing any 'annotations' entries in hook JSON to be silently ignored. Signed-off-by: W. Trevor King <wking@tremily.us>
The typo is a copy/paste error from 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, containers#686). Signed-off-by: W. Trevor King <wking@tremily.us>
Reported by Gary Edwards [1]. Both typos are originally from 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, containers#686). [1]: containers#884 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
We've had logrus logging in the monitor code since it landed in 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, containers#686). This commit adds similar logging to the initial hook.New() and Manager.Hooks() calls to make it easier to see if those are working as expected. Signed-off-by: W. Trevor King <wking@tremily.us>
Member
|
LGTM |
Member
Collaborator
|
📌 Commit 6ded615 has been approved by |
Collaborator
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 4, 2018
Reported by Gary Edwards [1]. Both typos are originally from 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). [1]: #884 (comment) Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
rh-atomic-bot
pushed a commit
that referenced
this pull request
Jun 4, 2018
We've had logrus logging in the monitor code since it landed in 68eb128 (pkg/hooks: Version the hook structure and add 1.0.0 hooks, 2018-04-27, #686). This commit adds similar logging to the initial hook.New() and Manager.Hooks() calls to make it easier to see if those are working as expected. Signed-off-by: W. Trevor King <wking@tremily.us> Closes: #887 Approved by: rhatdan
Member
|
LGTM, but FAH28 tests look to be flaking. |
Collaborator
|
☀️ Test successful - status-papr |
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
This typo from 68eb128 (#686) was causing any
annotationsentries in hook JSON to be silently ignored.This PR also fixes a number of other issues:
ReadDirwas silently ignoring missing hook executables. Now it bubbles those up.New(...)) or injection (behindHooks(...)). Now it has logging.versionand appropriate escaping for backslashes (oci-nvidia-hook not working as expected with podman command #884). Now these are fixed.When.Matchtests had a copy/pasted "both, and" name that should have been "both, or". Now it is "both, or".