Skip to content

Multiple slim#850

Merged
SimenB merged 2 commits intonodejs:masterfrom
LaurentGoderre:multiple-slim
Nov 29, 2018
Merged

Multiple slim#850
SimenB merged 2 commits intonodejs:masterfrom
LaurentGoderre:multiple-slim

Conversation

@LaurentGoderre
Copy link
Member

@LaurentGoderre LaurentGoderre commented Aug 27, 2018

No description provided.

@khasim
Copy link

khasim commented Aug 29, 2018

Hi @LaurentGoderre will this be fixed shortly, anything I can help with ?

Thanks
Khasim

@LaurentGoderre LaurentGoderre mentioned this pull request Sep 5, 2018
Closed
@LaurentGoderre LaurentGoderre force-pushed the multiple-slim branch 2 times, most recently from b29a16f to 9fc3dab Compare September 5, 2018 12:56
@LaurentGoderre LaurentGoderre changed the title WIP: Multiple slim Multiple slim Sep 5, 2018
@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Sep 5, 2018

@mkumatag

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Sep 5, 2018

The benefit of this is it creates the jessie-slim and stretch-slim tags and the default_variant controls the slim and the default tag. So when we switch default_variant to stretch slim will point to stretch-slim

@chorrell
Copy link
Contributor

chorrell commented Sep 5, 2018

I'm not sure what caused this test to stall: https://travis-ci.com/nodejs/docker-node/jobs/143886958

Maybe we need to pin to a specific version on eclint?

@chorrell
Copy link
Contributor

chorrell commented Sep 5, 2018

I restarted the test build.

@chorrell
Copy link
Contributor

chorrell commented Sep 5, 2018

Maybe that failure was just an issue with the npm registry? Anyway, it's nice that we can restart builds now :-)

chorrell

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Sep 5, 2018

@chorrell good catch!

chorrell

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre LaurentGoderre force-pushed the multiple-slim branch 2 times, most recently from 56b248a to 26fb32f Compare September 5, 2018 17:14
chorrell

This comment was marked as off-topic.

Sign in to view

chorrell

This comment was marked as off-topic.

Sign in to view

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Sep 6, 2018

@chorrell @PeterDaveHello what do you think of my approach?

@chorrell
Copy link
Contributor

chorrell commented Sep 6, 2018

I think I prefer separate templates. It will make the update script simpler and we can just delete the jessie template when we drop it.

@yosifkit
Copy link
Contributor

yosifkit commented Sep 11, 2018

This bit from buildpack-deps:curl works for all versions of Debian:

RUN set -ex; \
	if ! command -v gpg > /dev/null; then \
		apt-get update; \
		apt-get install -y --no-install-recommends \
			gnupg \
			dirmngr \
		; \
		rm -rf /var/lib/apt/lists/*; \
	fi

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Sep 12, 2018

@yosifkit wow that's even cleaner!!!!

@chorrell
Copy link
Contributor

chorrell commented Nov 21, 2018

LGTM. The only thing I’m not certain about are the architecture files

@tianon
Copy link
Contributor

tianon commented Nov 21, 2018

This seems all correct to me.

@chorrell
Copy link
Contributor

chorrell commented Nov 21, 2018 •
edited
Loading

Sounds like we can merge this then do the Docker Hub PR for #921

I'm still sick, will most likely be taking the rest of the day off. If someone else can pick this up, that would be great.

@lag-linaro
Copy link

lag-linaro commented Nov 22, 2018

Ok, even better now:

Yes, it's getting better.

Tags: 8.12.0-onbuild, 8.12-onbuild, 8-onbuild, carbon-onbuild
Architectures: arm32v7, amd64, i386

Why do these -onboard images not support the other architectures? Are these still built from Jessie?

(same comment for all of the other -onbuild images)

GitCommit: 526c6e6
Directory: 8/onbuild

Tags: 8.12.0-stretch-slim, 8.12-stretch-slim, 8-stretch-slim, carbon-stretch-slim, 8.12.0-slim, 8.12-slim, 8-slim, carbon-slim
Architectures: arm32v7, amd64, i386

Do you know why this doesn't support the other architectures? Are these still built from Jessie?

(same comment for all the other -slim variants)

GitCommit: ca73c15
Directory: 8/stretch-slim

Tags: 11.2.0-stretch-slim, 11.2-stretch-slim, 11-stretch-slim, stretch-slim, 11.2.0-slim, 11.2-slim, 11-slim, slim, slim

You have 2 'slim' entries in this line.

Architectures: arm32v7, arm64v8, amd64, ppc64le, s390x
GitCommit: ca73c15
Directory: 11/stretch-slim

@chorrell chorrell mentioned this pull request Nov 22, 2018
Closed
@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Nov 22, 2018

@lag-linaro the two slim was fixed. As for what architecture are supported, I just preserved what was there before.

@lag-linaro
Copy link

lag-linaro commented Nov 22, 2018 •
edited
Loading

Ah, so this change doesn't update the GitCommit tags.

Is that already set as a separate piece of work?

This library file is now referencing tags which are several months old.

This misses out on the fact that the -slim and -onbuild images have been updated to Stretch and the Alpine images have been upgraded to 3.8. Making them compatible with more architectures now.

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Nov 22, 2018

@lag-linaro perhaps but that should be looked at in a different PR I think

@lag-linaro
Copy link

lag-linaro commented Nov 23, 2018

I'm okay with that.

In which case, if the duplicate slim issue has been resolved:

LGTM

@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Nov 23, 2018

@SimenB want to merge?

@SimenB
Copy link
Member

SimenB commented Nov 24, 2018

I'm a bit hesitant to land this before the security releases on Tuesday... Thoughts on that?

@chorrell
Copy link
Contributor

chorrell commented Nov 24, 2018

The security update will include stretch as the new default for the slim and base image. Landing this first would give people a Jessie option for slim.

And we should do the pr for the recent update to v8.x.x before Tuesday

@chorrell
Copy link
Contributor

chorrell commented Nov 25, 2018

Actually, I think you're right. This can wait until after the security update

This was referenced Nov 28, 2018
Closed
Closed
Merged
@LaurentGoderre
Copy link
Member Author

LaurentGoderre commented Nov 28, 2018

@chorrell @PeterDaveHello @SimenB rebased

SimenB
SimenB approved these changes Nov 29, 2018
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

We'll still have node:10-slim right?

@chorrell
Copy link
Contributor

chorrell commented Nov 29, 2018 •
edited
Loading

Yep (via ./generate-stackbrew-library.sh):

# this file is generated via https://github.com/nodejs/docker-node/blob/fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9/generate-stackbrew-library.sh

Maintainers: The Node.js Docker Team <https://github.com/nodejs/docker-node> (@nodejs)
GitRepo: https://github.com/nodejs/docker-node.git

Tags: 8.14.0-jessie, 8.14-jessie, 8-jessie, carbon-jessie
Architectures: arm32v7, amd64, i386
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 8/jessie

Tags: 8.14.0-jessie-slim, 8.14-jessie-slim, 8-jessie-slim, carbon-jessie-slim
Architectures: arm32v7, amd64, i386
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 8/jessie-slim

Tags: 8.14.0-alpine, 8.14-alpine, 8-alpine, carbon-alpine
Architectures: arm32v6, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 8/alpine

Tags: 8.14.0-onbuild, 8.14-onbuild, 8-onbuild, carbon-onbuild
Architectures: arm32v7, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: 0aae692a71251b60c489c41b7b1f28daa05829e5
Directory: 8/onbuild

Tags: 8.14.0-stretch, 8.14-stretch, 8-stretch, carbon-stretch, 8.14.0, 8.14, 8, carbon
Architectures: arm32v7, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 8/stretch

Tags: 8.14.0-stretch-slim, 8.14-stretch-slim, 8-stretch-slim, carbon-stretch-slim, 8.14.0-slim, 8.14-slim, 8-slim, carbon-slim
Architectures: arm32v7, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 8/stretch-slim

Tags: 6.15.0-jessie, 6.15-jessie, 6-jessie, boron-jessie
Architectures: arm32v7, amd64, i386
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 6/jessie

Tags: 6.15.0-jessie-slim, 6.15-jessie-slim, 6-jessie-slim, boron-jessie-slim
Architectures: arm32v7, amd64, i386
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 6/jessie-slim

Tags: 6.15.0-alpine, 6.15-alpine, 6-alpine, boron-alpine
Architectures: amd64
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 6/alpine

Tags: 6.15.0-onbuild, 6.15-onbuild, 6-onbuild, boron-onbuild
Architectures: arm32v7, amd64, i386
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 6/onbuild

Tags: 6.15.0-stretch, 6.15-stretch, 6-stretch, boron-stretch, 6.15.0, 6.15, 6, boron
Architectures: arm32v7, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 6/stretch

Tags: 6.15.0-stretch-slim, 6.15-stretch-slim, 6-stretch-slim, boron-stretch-slim, 6.15.0-slim, 6.15-slim, 6-slim, boron-slim
Architectures: arm32v7, amd64, i386
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 6/stretch-slim

Tags: 11.3.0-alpine, 11.3-alpine, 11-alpine, current-alpine, alpine
Architectures: arm32v6, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 11/alpine

Tags: 11.3.0-stretch, 11.3-stretch, 11-stretch, current-stretch, stretch, 11.3.0, 11.3, 11, current, latest
Architectures: arm32v7, arm64v8, amd64, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 11/stretch

Tags: 11.3.0-stretch-slim, 11.3-stretch-slim, 11-stretch-slim, current-stretch-slim, stretch-slim, 11.3.0-slim, 11.3-slim, 11-slim, current-slim, slim
Architectures: arm32v7, arm64v8, amd64, ppc64le, s390x
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 11/stretch-slim

Tags: 10.14.0-jessie, 10.14-jessie, 10-jessie, dubnium-jessie, lts-jessie
Architectures: arm32v7, amd64
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 10/jessie

Tags: 10.14.0-jessie-slim, 10.14-jessie-slim, 10-jessie-slim, dubnium-jessie-slim, lts-jessie-slim
Architectures: arm32v7, amd64
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 10/jessie-slim

Tags: 10.14.0-alpine, 10.14-alpine, 10-alpine, dubnium-alpine, lts-alpine
Architectures: arm32v6, arm64v8, amd64, i386, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 10/alpine

Tags: 10.14.0-stretch, 10.14-stretch, 10-stretch, dubnium-stretch, lts-stretch, 10.14.0, 10.14, 10, dubnium, lts
Architectures: arm32v7, arm64v8, amd64, ppc64le, s390x
GitCommit: fd2bad889b28587d4722f69c4a51d025df2ae624
Directory: 10/stretch

Tags: 10.14.0-stretch-slim, 10.14-stretch-slim, 10-stretch-slim, dubnium-stretch-slim, lts-stretch-slim, 10.14.0-slim, 10.14-slim, 10-slim, dubnium-slim, lts-slim
Architectures: arm32v7, amd64
GitCommit: fb54d5ff5f1717b5e4f85b59b7a914c87204c7e9
Directory: 10/stretch-slim

Tags: chakracore-8.11.1, chakracore-8.11, chakracore-8
Architectures: amd64
GitCommit: 8ccd57c1457a1b47adc4d82f9fed9ad51ccef3c5
Directory: chakracore/8

Tags: chakracore-10.13.0, chakracore-10.13, chakracore-10, chakracore
Architectures: amd64
GitCommit: 69c8a5f448f46f9e34d7fb577eca79ba01f6864d
Directory: chakracore/10

@SimenB SimenB merged commit 8c0a9f2 into nodejs:master Nov 29, 2018
@SimenB SimenB mentioned this pull request Nov 29, 2018
Closed
@SimenB
Copy link
Member

SimenB commented Nov 29, 2018

docker-library/official-images#5125

@LaurentGoderre LaurentGoderre deleted the multiple-slim branch November 29, 2018 16:01
@chorrell chorrell mentioned this pull request Nov 29, 2018
Merged
autophagy pushed a commit to crate/official-images that referenced this pull request Dec 12, 2018
@autophagy
Node: Update Node.js v10.14.1 w/ Yarn v1.12.3
830ee6b 
See:

- nodejs/docker-node#944
- https://github.com/nodejs/node/releases/tag/v10.14.1
-
 https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V10.md#10.14.1

Also includes:

- nodejs/docker-node#942
- nodejs/docker-node#850
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nschonni nschonni nschonni left review comments

@SimenB SimenB SimenB approved these changes

@PeterDaveHello PeterDaveHello PeterDaveHello approved these changes

@Starefossen Starefossen Awaiting requested review from Starefossen

@rvagg rvagg Awaiting requested review from rvagg

@pesho pesho Awaiting requested review from pesho

+1 more reviewer

@chorrell chorrell chorrell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@LaurentGoderre @khasim @chorrell @yosifkit @lag-linaro @PeterDaveHello @tianon @SimenB @nschonni

Comments