Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

chore: adaptations to dockerFile. Bumping ci workflow version #462

Merged
merged 5 commits into from
Sep 15, 2023

Conversation

Bidon15
Copy link
Member

@Bidon15 Bidon15 commented Sep 11, 2023

Overview

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

Comment on lines +11 to +12
ARG UID=10001
ARG USER_NAME=celestia
Copy link
Member Author

Choose a reason for hiding this comment

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

Basically, we are aligning all DockerFiles into 1 style like we have in app and node, where there is a celestia user and not root anymore. Let me know if this is fine for the orchestrator-relayer

Copy link
Member

Choose a reason for hiding this comment

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

Sure :D Thanks

curl \
jq \
# Creates a user with $UID and $GID=$UID
&& adduser ${USER_NAME} \
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be failing in docker CI:

93 Usage: adduser [OPTIONS] USER [GROUP]
  #10 1.493 
  #10 1.493 Create new user, or add USER to GROUP
  #10 1.493 
  #10 1.493 	-h DIR		Home directory
  #10 1.493 	-g GECOS	GECOS field
  #10 1.493 	-s SHELL	Login shell
  #10 1.493 	-G GRP		Group
  #10 1.493 	-S		Create a system user
  #10 1.493 	-D		Don't assign a password
  #10 1.493 	-H		Don't create home directory
  #10 1.493 	-u UID		User id
  #10 1.493 	-k SKEL		Skeleton directory (/etc/skel)
  #10 ERROR: process "/bin/sh -c apk update && apk add --no-cache         bash         curl         jq     && adduser ${USER_NAME}         -D         -g ${USER_NAME}         -h ${CELESTIA_HOME}         -s /sbin/nologin         -u ${UID}" did not complete successfully: exit code:  1

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the review ❤️
I totally forgot to point the home directory actually....

Comment on lines +11 to +12
ARG UID=10001
ARG USER_NAME=celestia
Copy link
Member

Choose a reason for hiding this comment

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

Sure :D Thanks

rach-id
rach-id previously approved these changes Sep 11, 2023
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

utACK.

Thanks a lot for handling this. This PR can be merged after this one is merged: #459

@Bidon15
Copy link
Member Author

Bidon15 commented Sep 11, 2023

I've reverted for the time being back to 1.20 due to quic incompat with 1.21 in current main head
Can you please ping me again once #459 is merged? 🙏

@rach-id
Copy link
Member

rach-id commented Sep 11, 2023

Sure, let's keep it to v1.21 and I'll ping you when that gets merged

evan-forbes
evan-forbes previously approved these changes Sep 11, 2023
@rach-id
Copy link
Member

rach-id commented Sep 13, 2023

@Bidon15 this is ready, once the conflicts are resolved 🙏

Dockerfile Outdated Show resolved Hide resolved
@rach-id
Copy link
Member

rach-id commented Sep 15, 2023

@Bidon15 Sorry, i went and resolved the conflicts so that I can release today. Let me know if I missed something

@rach-id rach-id merged commit d7b3581 into celestiaorg:main Sep 15, 2023
@Bidon15 Bidon15 deleted the docker-fix branch September 18, 2023 08:15
@Bidon15
Copy link
Member Author

Bidon15 commented Sep 18, 2023

@Bidon15 Sorry, i went and resolved the conflicts so that I can release today. Let me know if I missed something

no worries. It looks good! Thanks for driving it home! You 🪨

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants