fix: Correctly call kubectl to get all known images of the node#9408
Closed
ComaVN wants to merge 1 commit into
Closed
fix: Correctly call kubectl to get all known images of the node#9408ComaVN wants to merge 1 commit into
ComaVN wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
The single quotes around the `jsonpath` expression are incorrect: they are only needed when called in a shell, not when using `os.exec` The result was that the first image returned had a `'` prepended to the name, and the last image had a `'` appended, meaning that those images would never be considered already loaded. issue GoogleContainerTools#4955
Contributor
|
Hi @ComaVN Thanks for the fix!! |
ComaVN
force-pushed
the
fix-4955-dont-reload-unchanged-images
branch
from
909cc0a to
939a520
Compare
Author
|
rebased to fix unrelated unit test failures |
Author
|
made redundant by #9449 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Fixes: #4955
Description
When checking if images are already present on the k8s node,
os.execis used, which doesn't go through any shell expansion, so it doesn't require quoting any arguments (quotes are passed through as-is).This means the first image returned had a
'prepended to the name, and the last image had a'appended, which causes issue #4955User facing changes (remove if N/A)
A few less unnecessary image reloads :)
Follow-up Work (remove if N/A)
There are a few more places where
jsonpath='(...)'shows up in anos.execcall, someone might want to check those. I noticed at least some of them were "fixed" by trimming the quotes from the output.