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

Shifting dependency versions in same release branch causing downstream issues #1598

Closed
akutz opened this issue Oct 17, 2019 · 8 comments · Fixed by #1637
Closed

Shifting dependency versions in same release branch causing downstream issues #1598

akutz opened this issue Oct 17, 2019 · 8 comments · Fixed by #1637
Assignees
Labels
area/dependency Issues or PRs related to dependency changes area/release Issues or PRs related to releasing kind/documentation Categorizes issue or PR as related to documentation. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.
Milestone

Comments

@akutz
Copy link
Contributor

akutz commented Oct 17, 2019

What steps did you take and what happened:
There seems to be some module weirdness originating out of the Cluster API Upgrade Tool centered around the github.com/Azure/go-autorest package.

The upgrade tool pulls in k8s.io/[email protected]+incompatible which does not import autorest. However, autorest shows up several times as a transitive dependency for the upgrade tool:

$ cd $(mktemp -d) && \
  git clone https://github.com/vmware/cluster-api-upgrade-tool . && \
  go list -m -u all 2>/dev/null | grep autorest
github.com/Azure/go-autorest/autorest v0.9.0 [v0.9.2]
github.com/Azure/go-autorest/autorest/adal v0.6.0 [v0.8.0]
github.com/Azure/go-autorest/autorest/date v0.2.0
github.com/Azure/go-autorest/autorest/mocks v0.3.0
github.com/Azure/go-autorest/logger v0.1.0
github.com/Azure/go-autorest/tracing v0.5.0

The upgrade tool is inheriting the transitive dependency on autorest via the direct dependency on sigs.k8s.io/[email protected]:

$ cd $(mktemp -d) && \
  git clone https://github.com/kubernetes-sigs/cluster-api . && \
  git checkout -b v0.2.1 v0.2.1 && \
  go list -m -u all 2>/dev/null | grep autorest
github.com/Azure/go-autorest/autorest v0.9.0 [v0.9.2]
github.com/Azure/go-autorest/autorest/adal v0.6.0 [v0.8.0]
github.com/Azure/go-autorest/autorest/date v0.2.0
github.com/Azure/go-autorest/autorest/mocks v0.3.0
github.com/Azure/go-autorest/logger v0.1.0
github.com/Azure/go-autorest/tracing v0.5.0

However, what happens when a downstream project imports both the upgrade tool and a newer version of Cluster API v1alpha2?

$ cd $(mktemp -d) && \
  git clone https://github.com/kubernetes-sigs/cluster-api . && \
  git checkout -b v0.2.5 v0.2.5 && \
  go list -m -u all 2>/dev/null | grep autorest
github.com/Azure/go-autorest v11.1.2+incompatible [v13.2.0+incompatible]

Conflict!

FWIW, the dependency on github.com/Azure/go-autorest is transitive via k8s.io/client-go.

Essentially, because Cluster API moves so quickly, and we're starting to see libraries, such as the upgrade tool, built on top of Cluster API, dependency conflicts are being created when downstream work wants to use both the most recent patch of Cluster API with a library built on top of Cluster API that still references an older version from the same release branch.

It makes me wonder if Cluster API should stop updating dependencies unless otherwise absolutely required during a release branch?

  • Cluster API v0.2.1 depends on k8s.io/client-go v11.0.1-0.20190409021438-1a26190bd76a+incompatible
  • Cluster API v0.2.5 depends on k8s.io/client-go v0.0.0-20190918200256-06eb1244587a

The result is a messy situation for people trying to use Cluster API and libraries built on top of Cluster API.

One thing we could do to combat this is to start adding comments to go.mod files such as:

// The following dependency was resolved from the tag "kubernetes-1.15.4"
k8s.io/client-go v0.0.0-20190918200256-06eb1244587a

With anything related to k/k, the above comment approach would simplify aligning all k/k related dependencies in a project that imports both Cluster API and libraries built on top of it.

What did you expect to happen:
A downstream consumer of Cluster API should be able to pull in a newer version of Cluster API from the same release branch, even when another dependency pulls in a different version of Cluster API from the same release branch.

cc @ncdc

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 17, 2019
@akutz
Copy link
Contributor Author

akutz commented Oct 17, 2019

/remove kind/bug

@ncdc
Copy link
Contributor

ncdc commented Oct 17, 2019

I need to fix the upgrade tool's cluster-api version. We addressed this in the most recent releases of CAP*.

@ncdc
Copy link
Contributor

ncdc commented Oct 17, 2019

Going forward, I would like to do a better job of staying on a fixed version of controller-runtime, controller-tools, client-go, apimachinery, etc., within a patch stream.

@akutz
Copy link
Contributor Author

akutz commented Oct 17, 2019

/remove-kind bug

@k8s-ci-robot k8s-ci-robot removed the kind/bug Categorizes issue or PR as related to a bug. label Oct 17, 2019
@akutz
Copy link
Contributor Author

akutz commented Oct 17, 2019

Hi @ncdc,

Thanks! Yeah, my concern is the going forward part -- hence the suggestion of the comments to make things easier for downstream consumers. But even then it will be tough to catch these as CAPI and its ecosystem grows. I wonder if some job should be set up to ensure there are no graph conflicts across a specified number of related repositories?

@vincepri
Copy link
Member

/area dependency
/area release

I'm not sure if this issue is directly actionable, it seems we might need a policy written somewhere in the repository, but that might be a different issue

@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/release Issues or PRs related to releasing labels Oct 17, 2019
@ncdc
Copy link
Contributor

ncdc commented Oct 23, 2019

We should write/update a reviewers guide around expectations within a patch stream.

@ncdc ncdc added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. labels Oct 23, 2019
@ncdc ncdc added this to the v0.3.0 milestone Oct 23, 2019
@ncdc ncdc self-assigned this Oct 23, 2019
@ncdc ncdc removed the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Oct 23, 2019
@ncdc
Copy link
Contributor

ncdc commented Oct 23, 2019

/lifecycle active

@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependency Issues or PRs related to dependency changes area/release Issues or PRs related to releasing kind/documentation Categorizes issue or PR as related to documentation. lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants