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.

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

bug: fixed dockerfile and dockerfile.contrib to use node:erbium-buster-slim #666

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

scyto
Copy link
Contributor

@scyto scyto commented Feb 20, 2021

dependabot changed all references to to node:erbium-buster-slim to node:15.9.0-alpine.
This PR reverts those changes.,

dependabot reverted the base image to alpine
this PR changes it back to node:erbium-buster-slim
@coveralls
Copy link

coveralls commented Feb 20, 2021

Pull Request Test Coverage Report for Build 585772392

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 19.388%

Totals Coverage Status
Change from base Build 581791076: 0.0%
Covered Lines: 2027
Relevant Lines: 10724

💛 - Coveralls

@scyto
Copy link
Contributor Author

scyto commented Feb 20, 2021

@jcam erbium-buster-slim is failing on the build check

#5 [linux/arm/v6 internal] load metadata for docker.io/library/node:erbium-buster-slim
#5 sha256:6d812dcf589ff23822c6d740c3f6297c73548bbd6c0973a3f85ef34d249e05c7
#5 ERROR: no match for platform in manifest sha256:958fe4aeac9e0295267b67f5fadbb1e724f7ba137d0ee5c6ad90214495bff374: not found

the github workflow expects linux/arm64,linux/amd64,linux/arm/v6,linux/arm/v7
node:erbium-buster-slim has tag for these linux/arm64/v8,linux/amd64,linux/arm/v7,linux/ppc64le,linux/s390x

options:

  1. we pick a different base buster and install node manually
  2. we change the github workflow to reflect all the architectures node:erbium-buster-slim has?
  3. we switch to node:erbium-alpine which does have v6 but then we would need to convert all APT commands to APK (assuming all the dependencies are even on alpine?)

I assume option 2 would break anyone relying on the v6 version we currently publish? unless that uses a different workflow?

@jcam
Copy link
Contributor

jcam commented Feb 20, 2021

What workflow is running the dockerfile.contrib?
I picked erbium-buster-slim because that was what we used to use here before dependabot broke it (the first time, back in january):
9767d76#diff-4ddc2e10398d5c169795b7937f893ec72d2619b395259c592156a5dd0b476c9f

@jcam
Copy link
Contributor

jcam commented Feb 20, 2021

the regular Dockerfile should be using node alpine

reverted dockerfile to 15.9.0-alpine
@scyto
Copy link
Contributor Author

scyto commented Feb 21, 2021

the regular Dockerfile should be using node alpine

ok will revert the changes to that file, why the difference?

@jcam
Copy link
Contributor

jcam commented Feb 21, 2021

contrib does a lot more :D

@jcam
Copy link
Contributor

jcam commented Feb 21, 2021

if you use slim up top, you'll need to run an apt-get install a few things... build-essential, git, jq... i think that's all

@scyto
Copy link
Contributor Author

scyto commented Feb 21, 2021

if you use slim up top, you'll need to run an apt-get install a few things... build-essential, git, jq... i think that's all

I put it back to alpine for the dockerfile but to erbirum-buster-slim (to mirror your change that got wiped away) for the .contrib file, i had (wrongly) assumed that both edits by the dependabot was bad.

I am not sure that i get you on the build-essentials, git and jq - i looked at your previous commit and it didn't have those?
what base build do you think we should have in Dockerfile.contrib - something other than slim?
oh i see, i put back to erbium-buster i don't need to go super deep yet on getting these as small as possible, though i always like that challenge :-)

change git clone section to use erbium-buster not erbium-buster-slim
change buster-slim to buster
put slim back after testing
@robertsLando robertsLando merged commit 7b93c28 into zwave-js:master Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants