Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

convert from govendor to dep #265

Merged
merged 7 commits into from
Jan 11, 2018
Merged

convert from govendor to dep #265

merged 7 commits into from
Jan 11, 2018

Conversation

bryanl
Copy link
Member

@bryanl bryanl commented Jan 4, 2018

Convert from govendor to dep.

note: this updates a few packages as well.

  • convert to dep
  • updating contrib docs

Signed-off-by: bryanl [email protected]

@bryanl bryanl requested a review from jessicayuen January 4, 2018 17:41
@bryanl bryanl self-assigned this Jan 4, 2018
@bryanl bryanl changed the title convert to dep [WIP] convert to dep Jan 4, 2018
@bryanl bryanl changed the title [WIP] convert to dep convert from govendor to dep Jan 6, 2018
@hausdorff
Copy link
Contributor

I think it's worth moving to dep, but it's hard for me to tell how confident we are in these changes.

  1. Does this PR complete the transition to dep? (I don't see a message like "Fixes Consider switching to dep from govendor #197".) If not, what remains?
  2. Did we resolve the problems Gus had when he tried to transition over ksonnet/kubecfg (as described in Consider switching to dep from govendor #197)?

Jessica Yao and others added 6 commits January 8, 2018 13:32
Signed-off-by: Jessica Yao <[email protected]>
Currently the component APIs are grouped into the same file as
manager.go. As more component APIs are added, managers.go will be
difficult to maintain. Similar to the environment and registry APIs,
component APIs will be moved into a separate file component.go

Signed-off-by: Jessica Yuen <[email protected]>
This adds a high level 'component' command and a 'component list'
command. 'component list' will pretty print all the components in
ksonnet application directory.

To accomplish this, an API is added to the metadata manager that returns
all components. Components are the individual files in /components, with
the path extension trimmed.

Signed-off-by: Jessica Yuen <[email protected]>
Signed-off-by: bryanl <[email protected]>
@bryanl
Copy link
Member Author

bryanl commented Jan 8, 2018

@hausdorff those issues haven't presented themselves in this change. Also, we can potentially update client-go to the latest release (6.x) and that should solve the dep/client-go issues raised in #197.

@bryanl
Copy link
Member Author

bryanl commented Jan 8, 2018

@hausdorff Also, yes this completes the transition to dep.

@hausdorff
Copy link
Contributor

@bryanl We should upgrade client-go, but I think we should do it in a different review. It's a pretty big operational risk (1) because we have accumulated a lot of ad hoc fixes for bugs in the current version, and (2) they do break the internal APIs all the time.

I'll take a look at the review in a bit.

@hausdorff
Copy link
Contributor

@bryanl Oh, also, if we're going to transition to dep and upgrade client-go, we should amortize that risk by making sure we upgrade kubecfg on both those things at the same time. Bitnami has to use that code in anger, and if there are issues, they will surface quite a bit faster when vetted through there.

@bryanl
Copy link
Member Author

bryanl commented Jan 8, 2018

The goal was to make the changes in steps. 1) move to dep 2) update client-go and friends. This PR Is only the first step.

@hausdorff
Copy link
Contributor

@bryanl ok, so, just to be clear: the plan is to update kubecfg as well, yes?

@hausdorff
Copy link
Contributor

Summarizing offline conversations: yes, we're planning on updating kubecfg.

Copy link
Contributor

@hausdorff hausdorff left a comment

Choose a reason for hiding this comment

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

Is it possible to dep in one commit, and then upgrade dependencies in another? I'm ok shipping this if we do that one thing.

@bryanl
Copy link
Member Author

bryanl commented Jan 10, 2018

@hausdorff I'm going to review to see if that is possible. dep and govendor. I may have to go back and adjust each dep by hand. The problem is explicit vs implicit dependencies.

@hausdorff
Copy link
Contributor

@bryanl Ok, if it's not easy, then I recommend just shipping it as-is. Another way to ameliorate this risk is to ship a release candidate for 0.9, so that at least people have a chance to test it in the wild. (I bring this up because we have a bunch of version-specific fixes for bugs in our dependencies, strung throughout the code, so. Ick.)

Copy link
Contributor

@hausdorff hausdorff left a comment

Choose a reason for hiding this comment

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

I'll pre-emptively vote "ship it", since one of those two scenarios will happen, and then it will get shipped either way.

@jessicayuen
Copy link
Contributor

Just incase you miss this, should do a rebase before merging. Some commits are dirty.

@jessicayuen
Copy link
Contributor

I've noticed this is taking a lot longer to build (even on subsequent builds). Are you seeing similar behavior @bryanl ?

➜  ksonnet git:(pr-265) ✗ time make
go build -o ks -ldflags="-X main.version=foo "  .
./docs/generate/update-generated-docs.sh
make  42.67s user 4.83s system 209% cpu 22.687 total

@bryanl
Copy link
Member Author

bryanl commented Jan 10, 2018 via email

@bryanl
Copy link
Member Author

bryanl commented Jan 11, 2018

@jessicayuen

on master:

go build -o ks -ldflags="-X main.version=dev-2018-01-11T11:21:03-0500 "  .
./docs/generate/update-generated-docs.sh
       28.90 real        47.02 user         8.39 sys

on current:

go build -o ks -ldflags="-X main.version=dev-2018-01-11T11:21:49-0500 "  .
./docs/generate/update-generated-docs.sh
       29.86 real        48.32 user         8.56 sys

@bryanl bryanl merged commit 3ca3e57 into ksonnet:master Jan 11, 2018
@bryanl bryanl deleted the switch-to-godep branch January 11, 2018 16:23
name = "k8s.io/client-go"
packages = ["discovery","discovery/fake","dynamic","kubernetes/scheme","kubernetes/typed/core/v1","pkg/api","pkg/api/install","pkg/api/v1","pkg/apis/apps","pkg/apis/apps/v1beta1","pkg/apis/authentication","pkg/apis/authentication/v1","pkg/apis/authentication/v1beta1","pkg/apis/authorization","pkg/apis/authorization/v1","pkg/apis/authorization/v1beta1","pkg/apis/autoscaling","pkg/apis/autoscaling/v1","pkg/apis/autoscaling/v2alpha1","pkg/apis/batch","pkg/apis/batch/v1","pkg/apis/batch/v2alpha1","pkg/apis/certificates","pkg/apis/certificates/v1beta1","pkg/apis/extensions","pkg/apis/extensions/v1beta1","pkg/apis/policy","pkg/apis/policy/v1beta1","pkg/apis/rbac","pkg/apis/rbac/v1alpha1","pkg/apis/rbac/v1beta1","pkg/apis/settings","pkg/apis/settings/v1alpha1","pkg/apis/storage","pkg/apis/storage/v1","pkg/apis/storage/v1beta1","pkg/util","pkg/util/parsers","pkg/version","plugin/pkg/client/auth","plugin/pkg/client/auth/gcp","plugin/pkg/client/auth/oidc","rest","rest/watch","testing","third_party/forked/golang/template","tools/auth","tools/clientcmd","tools/clientcmd/api","tools/clientcmd/api/latest","tools/clientcmd/api/v1","tools/metrics","transport","util/cert","util/clock","util/flowcontrol","util/homedir","util/integer","util/jsonpath"]
revision = "21300e3e11c918b8e6a70fb7293b310683d6c046"
version = "v3.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for 1.6 Kubernetes, does it mean that more recent features like CRDs are not supported?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants