Fix packaging tests after addition of new wolfi-based image #112831
Conversation
brianseeders
added
>non-issue
:Delivery/Packaging
breskeby
force-pushed
the
missing-wolfi
branch
4 times, most recently
from
2f5efe1 to
8199a19
Compare
breskeby
force-pushed
the
missing-wolfi
branch
from
d193877 to
9fad26f
Compare
|
Pinging @elastic/es-delivery (Team:Delivery) |
|
resolved packaging tests tested on buildkite: https://buildkite.com/elastic/elasticsearch-periodic-packaging/builds/3850 |
.buildkite/hooks/pre-command
Outdated
| @@ -83,6 +83,33 @@ if [[ "${USE_PROD_DOCKER_CREDENTIALS:-}" == "true" ]]; then | |||
|
|
|||
| DOCKER_REGISTRY_PASSWORD="$(vault read -field=password secret/ci/elastic-elasticsearch/migrated/prod_docker_registry_credentials)" | |||
| export DOCKER_REGISTRY_PASSWORD | |||
|
|
|||
There was a problem hiding this comment.
Can you just do docker login --username "$DOCKER_REGISTRY_USERNAME" --password "$DOCKER_REGISTRY_PASSWORD" docker.elastic.co instead of all of this stuff?
There was a problem hiding this comment.
(and by "can you" I mean: do you know if it works?)
There was a problem hiding this comment.
@brianseeders I tried that and the problem with that is, that we run this logic on all OS types, even those which don't have docker installed. using this file only approach allows not failing on those systems. Alternatively we could just not fail if docker is not available but that approach did not seem better than this file based one
There was a problem hiding this comment.
In that case, I would do:
if which docker > /dev/null 2>&1; then
docker login --username "$DOCKER_REGISTRY_USERNAME" --password "$DOCKER_REGISTRY_PASSWORD" docker.elastic.co
fi
which has the advantage of not blowing away the existing docker config if there is one.
And probably move the vault lookups into the if as well, since they're useless on OSes that don't have docker, right?
There was a problem hiding this comment.
I would approve the PR but you'll have to since I opened it
brianseeders
changed the title
breskeby
added
auto-merge-without-approval
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…112831) * Add more missing wolfi references to fix tests * packaging tests require access to docker registry * Fix symlink for es distributions jdk cacerts in wolfi docker * Fix native support on wolfi images * Fix provided keystore packaging tests for wolfi * Add utils used for testing to wolfi image * Explicitly set default shell to bash in docker images * Fix docker config issues * Apply review feedback around docker login --------- Co-authored-by: Rene Groeschke <[email protected]> (cherry picked from commit e9b3033)
No description provided.