cbscore: support local registries and container reuse#29
cbscore: support local registries and container reuse#29UweSchwaeke wants to merge 2 commits intoclyso:mainfrom
Conversation
UweSchwaeke
force-pushed
the
wip-cbsbuild-handle-local-registry
branch
from
a1d3d68 to
ccf7069
Compare
UweSchwaeke
force-pushed
the
wip-cbsbuild-handle-local-registry
branch
2 times, most recently
from
2cd5da2 to
5b9f667
Compare
| ) | ||
| if rc != 0: | ||
| logger.debug(stdout) | ||
| if rc == 2 and re.search("already installed", stderr): |
There was a problem hiding this comment.
magic number here. Maybe use the 'errno module? according to errno, '2' will be "ENOENT":
1 = EPERM
2 = ENOENT
So, import errno and use errno.ENOENT here?
There was a problem hiding this comment.
interestingly, I wouldn't expect rpm to return ENOENT in this case though. Are we absolutely sure it's a 2 being returned here instead of a 17 (i.e., EEXIST)?
There was a problem hiding this comment.
Nope, turns out this is not an error code -- instead, this is "the number of failed operations". E.g.,
➜ joao@behemoth cbs.git git:(wip/ceph-debug-component) ✗ sudo rpm -Uvh $(seq 1 3); echo $?
error: open of 1 failed: No such file or directory
error: open of 2 failed: No such file or directory
error: open of 3 failed: No such file or directory
3
So we can't reliably rely on the return code. We must only rely on the error message and the fact that rc != 0.
There was a problem hiding this comment.
this behavior of rpm is awlward, still 2 might be the right retcode here becase we don't install 1\n2\n3.
nevertheless i removed made a precondition to only install it if rpm -q cosign doesn't return 0.
cbscore/src/cbscore/images/skopeo.py
Outdated
| msg = f"error inspecting image '{img}': {err}" | ||
| logger.error(msg) | ||
| if re.match(r".*not\s+found.*", err): | ||
| if retcode == 2 or re.match(r".*not\s+found.*", err): |
There was a problem hiding this comment.
same here as before, use errno.ENOENT
There was a problem hiding this comment.
okay, here using errno.ENOENT makes sense, unlike the comment on the other patch.
There was a problem hiding this comment.
i can change it, but skopeo uses 2 as a return value instead of the ENOENT. I know its bad to use magic numbers but if we give them a meaning than i would prefer to make a constant out of it, instead of ENOENT.
Replaced it with SKOPEO_IMAGE_NOT_FOUND_RETCODE
There was a problem hiding this comment.
ENOENT is 2 though, is it not?
There was a problem hiding this comment.
sure but skopeo uses the magic number 2 not os.ErrNotExist. thats the reason why i thought its not good to rely on ENOENT but i will change it
There was a problem hiding this comment.
changed to ENOENT
| ) | ||
| if rc != 0: | ||
| logger.debug(stdout) | ||
| if rc == 2 and re.search("already installed", stderr): |
There was a problem hiding this comment.
Nope, turns out this is not an error code -- instead, this is "the number of failed operations". E.g.,
➜ joao@behemoth cbs.git git:(wip/ceph-debug-component) ✗ sudo rpm -Uvh $(seq 1 3); echo $?
error: open of 1 failed: No such file or directory
error: open of 2 failed: No such file or directory
error: open of 3 failed: No such file or directory
3
So we can't reliably rely on the return code. We must only rely on the error message and the fact that rc != 0.
cbscore/src/cbscore/images/skopeo.py
Outdated
| msg = f"error inspecting image '{img}': {err}" | ||
| logger.error(msg) | ||
| if re.match(r".*not\s+found.*", err): | ||
| if retcode == 2 or re.match(r".*not\s+found.*", err): |
There was a problem hiding this comment.
okay, here using errno.ENOENT makes sense, unlike the comment on the other patch.
UweSchwaeke
force-pushed
the
wip-cbsbuild-handle-local-registry
branch
4 times, most recently
from
3902936 to
3e9ca43
Compare
UweSchwaeke
force-pushed
the
wip-cbsbuild-handle-local-registry
branch
2 times, most recently
from
e1a2f11 to
636cac6
Compare
* what: if the return code of the rpm process is 2, check if the failure reason is that the package is already installed. * why: when reusing a container, the package might already be present. this occurs when a build runner job must be debugged. Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
* what: add option --tls-verify to subcommands build and runner build. pass the tls-verify flag to skopeo when querying the registry. check if the return value from skopeo inspect equals "not found" (exit code 2). * why: if the image is pushed to a local container registry with a self-signed certificate, skopeo must not verify the certificate to avoid errors. current versions of skopeo (1.20.0) return exit code 2 if an image is not found. Signed-off-by: Uwe Schwaeke <uwe.schwaeke@clyso.com>
UweSchwaeke
force-pushed
the
wip-cbsbuild-handle-local-registry
branch
from
636cac6 to
98d78f8
Compare
2 participants
what:
add flag to bypass tls certificate verification for skopeo. check
skopeo return code when finding an image on the registry.
ignore rpm install failure if the package is already installed.
why:
local container registries don't need valid tls certificates or may
use self-signed ones. skopeo verifies certificates by default unless
--tls-verify=false is passed.
note:
this also makes the container reusable for debugging. currently,
rpm install fails with return code 2 if the package is already installed.
in a production environment, containers are generated from scratch,
so this issue does not arise.