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

Remove dependencies on k8s.io/apimachinery #225

Closed
wants to merge 3 commits into from

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jan 23, 2017

Per #207 (comment) , depending on the latest versions of k8s.io makes this code very difficult to use in openshift/origin. So, drop dependencies on k8s.io/apimachinery/pkg/util/{error,net} by inlining or copying the necessary parts.

(This leaves k8s.io/pkg/util/homedir, which seems not to cause issues right now.)

Instead, copy the error.NewAggregate implementation inside.

This allows use of this package in OpenShift, which uses an older
version of k8s.io packages.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Instead, inline net.SetTransportDefaults and copy
net.NewProxierWithNoProxyCIDR.

This allows use of this package in OpenShift, which uses an older
version of k8s.io packages.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@runcom
Copy link
Member

runcom commented Jan 23, 2017

👍

Approved with PullApprove

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 23, 2017

… broken again, by kubernetes/kubernetes@335ef74 .

@erikh
Copy link
Contributor

erikh commented Jan 23, 2017 via email

@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 23, 2017

A well defined, centralized dep list would go a long way towards stability in the projects (due to matched deps) and also make it easier to isolate problems.

While I would certainly like not having to care, the trouble is that we don’t want to freeze the versions forever; if this did succeed and we had 20 projects using a centralized dependency list, it would in turn be politically very difficult to change the list in the future. The various upstreams do add quite a few bug fixes, not only breaking changes, and we do want to get those bug fixes with little hassle :)

Sharing a dependency list between skopeo and containers/image does sound very attractive, but with that we would probably have not encountered the recent breaking changes for a good few months, if not years—which would not make the porting work smaller, it would only aggregate it in time to a single week (and probably a time when it would be much more urgent than now, when we are haunted only by our own CI).

I talked to alex on the vndr project and he told me vndr cannot do subsets of repositories; making it so that we have to import all of docker/docker for docker/pkg/term.

That may be misleading: with vndr, one has to record an entire repository in vendor.conf, and perhaps all of that repository is pulled into a temporary directory, but the resulting vendor/github.com/docker directory is pruned to only contain the actually used subpackages.

@erikh
Copy link
Contributor

erikh commented Jan 23, 2017 via email

@erikh
Copy link
Contributor

erikh commented Jan 23, 2017 via email

@LK4D4
Copy link

LK4D4 commented Jan 23, 2017

@erikh everything unused is pruned, but what is used has the same version within project, so all deps which starts from i.e. github.com/coreos/etcd will have same version.

@erikh
Copy link
Contributor

erikh commented Jan 23, 2017 via email

…o/client-go

Because this is an entirely new package, depending on any version of it
should hopefully not be problematic for downstreams.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
@mtrmac
Copy link
Collaborator Author

mtrmac commented Jan 23, 2017

Updated to also handle the k8s.io/kubernetes/pkg/util/homedir move. Though, that still doesn’t pass tests — I guess we will, in the end, have to end up with a single PR combining this and #223 .

@runcom
Copy link
Member

runcom commented Jan 30, 2017

this has been included in #223

@runcom runcom closed this Jan 30, 2017
@mtrmac mtrmac deleted the no-k8s.io-apimachinery branch January 30, 2017 17:07
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.

None yet

4 participants