Bump Helm to v2.10.0 #82
Conversation
| - pkg/nstrvals | ||
| - pkg/helm | ||
| - pkg/tlsutil | ||
| - pkg/proto/hapi/release |
There was a problem hiding this comment.
I'm not entirely sure if this is good thing to do, but tried my best not to install unnecessary subpackages to vendor/ while building the project.
| - name: k8s.io/apimachinery | ||
| version: 01bc873149a1802eb74df583613872d126449ed5 | ||
| version: f6313580a4d36c7c74a3d845dda6e116642c4f90 |
There was a problem hiding this comment.
After running glide update, I've manually updated this to the version that is also seen in helm's glide.lock.
From my observation, it was required in order to fix build errors due to version mismatch between apimachinery and json-iterator.
There was a problem hiding this comment.
According to the discussion on the k8s slack, I'm going to add respective version: ... settings to affected dependencies, so that we won't need to manually update these. Thx for your review @databus23!
| - pkg/strvals | ||
| - pkg/sympath | ||
| - pkg/tlsutil | ||
| - pkg/urlutil | ||
| - pkg/version | ||
| - name: k8s.io/kubernetes |
There was a problem hiding this comment.
Dislike the huge additional dependency here, but glide update added it anyway.
There was a problem hiding this comment.
Urks. How did the helm people managed to bring that back in... :(
There was a problem hiding this comment.
Ah man. It seems the only reason k8s.io/kubernetes is pulled in is because of fake.go in k8s.io/helm/pkg/helm. And only because they use a single error from the k8s.io/helm/pkg/storage package in there. This is so unnecessary...
There was a problem hiding this comment.
This is clearly an accident. We should try to fix this upstream. We have had this before. helm/helm#2685
There was a problem hiding this comment.
So just by replacing storage.ErrReleaseNotFound(rlsName) with fmt.Errorf("release: %q not found", rlsName) three times in fake.go the kubernetes dependency goes away from k8s.io/pkg/helm
|
So I opened helm/helm#4499. Lets wait for on initial reaction on the PR. If the helm maintainers accept the changes we can fork the dep and backport the change until a new helm release incorporates the changes (Thats what I did before) |
|
@databus23 Awesome! And the plan looks great. Thanks for your support. |
Ran `glide update` and `glide install --strip-vendor`. Added version specifications to k8s.io/apimachienery and k8s.io/apiserver according to helm's glide.lock https://github.com/helm/helm/blob/cc7cc1087f586f64d0b0d08aa516a003ced2d85b/glide.lock#L457 to prevent build errors. The huge k8s.io/kubernetes dependency is introduced due to the upstream issue, helm/helm#4499. It should be changed to our own fork, and the removed in the future once the upstream incorporates the fix. Resolves databus23#81
|
So I'm going to merge this to resolve #81 for now. |
Ran
glide updateandglide install --strip-vendor, followed by manual changes toglide.lockto revertk8s.io/*pkg versions to those seen in helm's glide.lock https://github.com/helm/helm/blob/cc7cc1087f586f64d0b0d08aa516a003ced2d85b/glide.lock#L457 to avoid build errors.Resolves #81