Pin dependency library versions #154

2uasimojo · 2020-04-28T13:58:06Z

To make image builds more predictable, this commit pins the versions of
the two libraries pulled in by the docker build:

util-linux
amazon-efs-utils

This way we don't get surprised by changes in the underlying libs -- we
move along explicitly and deliberately.

Is this a bug fix or adding new feature?
Neither, just hardening the image build process.

What is this PR about? / Why do we need it?

Recently when trying to debug a problem mounting access points (see #152) one of our wild goose chases involved figuring out what version of amazon-efs-utils was built into the driver image at various times during our testing. Having the version pinned in the dockerfile for a given driver release would have removed the guesswork.

And in general this is just a good idea.

What testing is done?
Local make image-release, run (overriding entrypoint with a shell), verify installed versions with yum.

k8s-ci-robot · 2020-04-28T13:58:14Z

Hi @2uasimojo. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jharrington22 · 2020-04-28T14:14:46Z

Dockerfile

@@ -18,7 +18,7 @@ ADD . .
 RUN make

 FROM amazonlinux:2
-RUN yum install util-linux amazon-efs-utils -y
+RUN yum install util-linux-2.30.2-2.amzn2.0.4.x86_64 amazon-efs-utils-1.24-4.amzn2.noarch -y


Moving forward these versions may be dropped from the repository. Do we need to pin the FROM image too? Assuming the repo source URLs are specific for minor releases otherwise it wouldn't help.

Right, it doesn't seem to help, for example try base image 20200207 released prior to 1.24 and it still needs to get updates from the amzn2-core repository where only 1 version is available:

sh-4.2# yum list available amazon-efs-utils Loaded plugins: ovl, priorities Available Packages amazon-efs-utils.noarch 1.24-4.amzn2 amzn2-core

So any time a new package of amazon-efs-utils replaces the old, builds will fail (like as part of PR CI)? That will be painful for contributors.

Alternatively, we could log the amazon-efs-utils version with mount.efs --version

aws-efs-csi-driver/pkg/driver/version.go

Line 46 in be96b1d

func GetVersionJSON() (string, error) {

to help with the debugging/guesswork part of this issue.

It sounds like there's not a perfect answer here; we're going to have to choose the lesser evil.

I think we really only need to worry about the dep versions being removed (do they really do that?) during the lifecycle of latest. Once we tag a driver release, we're effectively freezing at whatever's available at that time; the bits stick to the image even if the repo is changed. If something happens where we need to update a tagged version (backporting a bugfix, for example), we can bump the lib versions at the same time if necessary -- the point being that we're always using a specific known working version.

And yes, developers working on latest could be inconvenienced if a lib version goes away, but IMO that's a good thing. As it stands, if something breaks while I'm developing, I don't know whether it's my code or because the anonymous lib version moved underneath me, and that's hard to discover. If the library version is pinned, that source of error is eliminated. If that version disappears, the failure (in CI or locally) is easy to understand. Yes, it requires a new/separate PR to fix, but IMO that's worth the trouble.

I forgot to set --show-duplicates:

$ yum --showduplicates list amazon-efs-utils Loaded plugins: priorities, update-motd Installed Packages amazon-efs-utils.noarch 1.25-2.amzn2 @amzn2-core Available Packages amazon-efs-utils.noarch 1.0-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.1-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.2-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.3-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.4-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.5-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.6-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.7-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.10-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.18-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.21-2.amzn2 amzn2-core amazon-efs-utils.noarch 1.23-2.amzn2 amzn2-core amazon-efs-utils.noarch 1.24-2.amzn2 amzn2-core amazon-efs-utils.noarch 1.24-3.amzn2 amzn2-core amazon-efs-utils.noarch 1.24-4.amzn2 amzn2-core amazon-efs-utils.noarch 1.25-1.amzn2 amzn2-core amazon-efs-utils.noarch 1.25-2.amzn2 amzn2-core amazon-efs-utils.noarch 1.25-3.amzn2 amzn2-core

So my point is moot. They don't seem to remove old versions.

leakingtapan · 2020-04-28T23:24:41Z

/ok-to-test

2uasimojo · 2020-04-29T16:20:05Z

/retest

2uasimojo · 2020-04-29T18:05:36Z

Sorry for my ignorance @leakingtapan: I can't tell if this build failure is spurious, or if it's happening because the checksum of the driver image has changed. The latter would be expected -- but I could use some help identifying where the expected checksum would need to be updated. (And I might also submit a PR to put that explanation into the text for this build failure to help future me.)

2uasimojo · 2020-04-29T18:06:19Z

/test pull-aws-efs-csi-driver-e2e
...to see if this is for real.

2uasimojo · 2020-04-29T19:42:12Z

Sorry for my ignorance @leakingtapan: I can't tell if this build failure is spurious, or if it's happening because the checksum of the driver image has changed.

Spurious. It passed on a retest. Disregard.

...as [suggested](kubernetes-sigs#154 (comment)).

2uasimojo · 2020-05-18T14:55:37Z

/test pull-aws-efs-csi-driver-e2e

2uasimojo · 2020-05-18T14:56:36Z

I've rebased and added a commit to pin the amazonlinux image as well. Can we continue this discussion?

2uasimojo · 2020-05-18T15:00:40Z

/test pull-aws-efs-csi-driver-e2e

2uasimojo · 2020-05-18T15:06:40Z

The pull-aws-efs-csi-driver-e2e job seems unhappy, but I don't think it's my PR. I'll wait a bit before retesting.

To make image builds more predictable, this commit pins the versions of the two libraries pulled in by the docker build: - util-linux - amazon-efs-utils This way we don't get surprised by changes in the underlying libs -- we move along explicitly and deliberately.

...as [suggested](kubernetes-sigs#154 (comment)).

wongma7 · 2020-06-24T23:36:09Z

/lgtm
/approve

I'm going to create a follow up PR to bump to 1.25-3 as that's the latest and I want to get that tested ASAP.

k8s-ci-robot · 2020-06-24T23:36:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, wongma7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [wongma7]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

2uasimojo · 2020-06-25T20:08:36Z

Thanks @wongma7

I'm going to create a follow up PR to bump to 1.25-3 as that's the latest and I want to get that tested ASAP.

Cool, would you please @ me when you do? I'd like to watch that and test with it as well.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 28, 2020

k8s-ci-robot requested review from jsafrane and wongma7 April 28, 2020 13:58

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Apr 28, 2020

jharrington22 reviewed Apr 28, 2020

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 28, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2020

2uasimojo force-pushed the pin-lib-versions branch from 47fc149 to dfe1e70 Compare May 18, 2020 14:37

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 18, 2020

2uasimojo added a commit to 2uasimojo/aws-efs-csi-driver that referenced this pull request May 18, 2020

Pin amazonlinux version

46bfe6e

...as [suggested](kubernetes-sigs#154 (comment)).

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2020

2uasimojo added 2 commits May 27, 2020 09:37

Pin amazonlinux version

5bd5bae

...as [suggested](kubernetes-sigs#154 (comment)).

2uasimojo force-pushed the pin-lib-versions branch from 46bfe6e to 5bd5bae Compare May 27, 2020 14:38

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2020

k8s-ci-robot assigned wongma7 Jun 24, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 24, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 24, 2020

k8s-ci-robot merged commit 841ed37 into kubernetes-sigs:master Jun 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pin dependency library versions #154

Pin dependency library versions #154

2uasimojo commented Apr 28, 2020

k8s-ci-robot commented Apr 28, 2020

jharrington22 Apr 28, 2020 •

edited

Loading

wongma7 May 4, 2020

2uasimojo May 18, 2020

wongma7 Jun 24, 2020

leakingtapan commented Apr 28, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

wongma7 commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

2uasimojo commented Jun 25, 2020

Pin dependency library versions #154

Pin dependency library versions #154

Conversation

2uasimojo commented Apr 28, 2020

k8s-ci-robot commented Apr 28, 2020

jharrington22 Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

wongma7 May 4, 2020

Choose a reason for hiding this comment

2uasimojo May 18, 2020

Choose a reason for hiding this comment

wongma7 Jun 24, 2020

Choose a reason for hiding this comment

leakingtapan commented Apr 28, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented Apr 29, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

2uasimojo commented May 18, 2020

wongma7 commented Jun 24, 2020

k8s-ci-robot commented Jun 24, 2020

2uasimojo commented Jun 25, 2020

jharrington22 Apr 28, 2020 •

edited

Loading