WASdev / ci.docker.websphere-traditional

Dockerfiles for WebSphere Application Server traditional
Apache License 2.0
171 stars 192 forks source link

Apply props in alphabetic order, new offline mode build, 8.5.x folder #143

Closed DSTOLF closed 5 years ago

DSTOLF commented 5 years ago
arthurdm commented 5 years ago

@arturdzm - can you please review this PR?

DSTOLF commented 5 years ago

I think the PR is ready for review again. If it's too many changes for on PR, please let me know and I'll break it down a bit. Here's a list of the changes in docker-build folder:

arthurdm commented 5 years ago

thanks so much for this contribution @DSTOLF! We really appreciate the time it took to create it.

My only comment is if we can make the userID and groupID configurable arguments, with defaults of 1001:0, to match our Liberty docker image (https://github.com/WASdev/ci.docker/blob/master/ga/18.0.0.4/kernel/Dockerfile#L23). That way you can easily switch to 510:500 if you wanted.

DSTOLF commented 5 years ago

Hi @arthurdm

Unfortunately, the userID and groupID can't be configurable arguments, as the COPY command on the Dockerfile doesn't have variable expansion.

If you want, I can change it to 1001:0, but whatever the IDs are, they'll be hardcoded. I suppose I can come up with some regex to easily change that, but I don't think you'll want to have that in your repo.

Please let me know how you want it done.

Regards, Daniel Stolf

arthurdm commented 5 years ago

@DSTOLF - ah, interesting - I wasn't aware of that limitation of COPY...thx for pointing that out! I have now added a +1 to the GH issue.

Until docker is fixed, if you could change it to be 1001:0, that would be great.

@arturdzm -after the uID/gID changes, this PR looks good to me. Can you please verify?

DSTOLF commented 5 years ago

@arthurdm

Just to be sure, it's uid=1001 and gid=0, right?

If so, all the Dockerfiles that I have changed to multi-stage build are fixed now.

9.0.0.9/Dockerfile and 8.5.x/Dockerfile (which was based on 9.0.0.9) are not multi-stage, so I didn't see any reason fiddle with uid and groupid there.

Regards!

arturdzm commented 5 years ago

@DSTOLF
Thank you for the update, yes uid,gid are correct now.

The rest looks good @arthurdm

arthurdm commented 5 years ago

awesome. thanks so much @DSTOLF!