Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DOCKER] add apt update to test fixture krb5kdc #565

Merged
merged 2 commits into from
Apr 15, 2021
Merged

[DOCKER] add apt update to test fixture krb5kdc #565

merged 2 commits into from
Apr 15, 2021

Conversation

nknize
Copy link
Collaborator

@nknize nknize commented Apr 15, 2021

This PR adds an apt update to the krb5kdc test fixture docker file.

@nknize
[DOCKER] add apt update to test fixture krb5kdc
33299a1 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize nknize added the >test-failure Test failure from CI, local build, etc. label Apr 15, 2021
@nknize nknize requested a review from peterzhuamazon April 15, 2021 19:39
@nknize
Copy link
Collaborator Author

nknize commented Apr 15, 2021

start gradle check

@nknize
Add apt upgrade -y
9b72b0f 
Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
@nknize
Copy link
Collaborator Author

nknize commented Apr 15, 2021

start gradle check

@odfe-release-bot
Copy link

✅   Gradle Wrapper Validation success 33299a1

@odfe-release-bot
Copy link

✅   DCO Check Passed 33299a1

@odfe-release-bot
Copy link

✅   Gradle Wrapper Validation success 9b72b0f

@odfe-release-bot
Copy link

✅   DCO Check Passed 9b72b0f

@odfe-release-bot
Copy link

✅   Gradle Precommit success 33299a1

@odfe-release-bot
Copy link

✅   Gradle Precommit success 9b72b0f

@odfe-release-bot
Copy link

❌   Gradle Check failure 33299a1
Log 90

Reports 90

@odfe-release-bot
Copy link

✅   Gradle Check success 9b72b0f
Log 91

Reports 91

@peterzhuamazon
Copy link
Member

peterzhuamazon commented Apr 15, 2021 •
edited
Loading

Hi @nknize,

Recommend using apt-get than apt due to:

WARNING: apt does not have a stable CLI interface yet. Use with caution in scripts.

apt is for terminal with beautiful outputs.
apt-get is for scripts and give stable, parsable outputs.

Thanks.

mihirsoni
mihirsoni approved these changes Apr 15, 2021
View reviewed changes
Copy link

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !!

test/fixtures/krb5kdc-fixture/Dockerfile
Comment on lines +2 to +3
RUN apt update -y
RUN apt upgrade -y
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np: may be we can combine in single Docker statement ? Doesn't matter as it is test but could save some size in docker.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx!! I'll probably also make a follow on change to use apt-get instead of apt

@nknize nknize merged commit 4dde0f2 into opensearch-project:main Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mihirsoni mihirsoni mihirsoni approved these changes

@peterzhuamazon peterzhuamazon Awaiting requested review from peterzhuamazon

Assignees
No one assigned
Labels
>test-failure Test failure from CI, local build, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nknize @odfe-release-bot @peterzhuamazon @mihirsoni