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

Bump k8s.io/kubernetes to 1.15.0 #4719

Merged
merged 2 commits into from
Jul 25, 2019

Conversation

josedonizetti
Copy link
Member

fix for #4656

still trying to figure out a couple of things here.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 9, 2019
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@josedonizetti
Copy link
Member Author

josedonizetti commented Jul 9, 2019

A storage-provisioner dependency breaks if we bump k8s. https://github.com/r2d4/external-storage/blob/master/lib/controller/controller.go#L46
I'm looking into it.

@medyagh
Copy link
Member

medyagh commented Jul 9, 2019

I am so excited for this PR !

@tstromberg
Copy link
Contributor

tstromberg commented Jul 9, 2019

A storage-provisioner dependency breaks if we bump k8s. https://github.com/r2d4/external-storage/blob/master/lib/controller/controller.go#L46
I'm looking into it.

Do we need the r2d4/external-storage fork any more? It isn't maintained, so if we can, it would be nice to switch to the upstream: https://github.com/kubernetes-incubator/external-storage

@josedonizetti
Copy link
Member Author

josedonizetti commented Jul 9, 2019

@tstromberg Yup, we gonna switch. The r2d4/external-storage is 1506 commits behind.

I'm just looking the 18 commits that were done in this repo if maybe one of them we should try to move upstream.

@tstromberg
Copy link
Contributor

@tstromberg Yup, we gonna switch. The r2d4/external-storage is 1506 commits behind.

I'm just looking the 18 commits that were done in this repo if maybe one of them we should try to move upstream.

Excellent. If nothing jumps out, I'd just switch implementations and see if the integration tests pass. If the feature it provided really was important, it would have test coverage =)

/cc @r2d4 in case he has any background to add here.

@medyagh
Copy link
Member

medyagh commented Jul 9, 2019

/retest this please

@tstromberg
Copy link
Contributor

Any luck with dropping the dependency on the r2d4 fork?

@tstromberg
Copy link
Contributor

Related: #3628

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: josedonizetti
To complete the pull request process, please assign tstromberg
You can assign the PR to them by writing /assign @tstromberg in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 24, 2019
@josedonizetti josedonizetti changed the title WIP: Bump k8s.io/kubernetes to 1.15.0 Bump k8s.io/kubernetes to 1.15.0 Jul 24, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2019
@josedonizetti josedonizetti changed the title Bump k8s.io/kubernetes to 1.15.0 WIP: Bump k8s.io/kubernetes to 1.15.0 Jul 24, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2019
@medyagh
Copy link
Member

medyagh commented Jul 24, 2019

seems to be an api change

--- FAIL: TestParseFeatureArgs (0.00s)
    --- FAIL: TestParseFeatureArgs/only_kubeadm_feature (0.00s)
        versions_test.go:148: Kubeadm Actual: map[], Expected: map[Auditing:true SelfHosting:false]
        versions_test.go:152: Component Actual: Auditing=true,SelfHosting=false, Expected: 
    --- FAIL: TestParseFeatureArgs/between_component_and_kubeadm_feature (0.00s)
        versions_test.go:148: Kubeadm Actual: map[], Expected: map[Auditing:true SelfHosting:false]
        versions_test.go:152: Component Actual: Auditing=true,PodPriority=true,SelfHosting=false,Accelerators=false, Expected: PodPriority=true,Accelerators=false

@josedonizetti
Copy link
Member Author

@minikube-bot Ok to test

@josedonizetti
Copy link
Member Author

@medyagh can you trigger minikube integration tests for this PR? pls

@josedonizetti josedonizetti changed the title WIP: Bump k8s.io/kubernetes to 1.15.0 Bump k8s.io/kubernetes to 1.15.0 Jul 24, 2019
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 24, 2019
@medyagh
Copy link
Member

medyagh commented Jul 24, 2019

@minikube-bot OK to test

@medyagh
Copy link
Member

medyagh commented Jul 25, 2019

Looks great ! thank you for doing this !

@medyagh medyagh merged commit d07dd39 into kubernetes:master Jul 25, 2019
@josedonizetti josedonizetti deleted the bump-k8s.io-kubernetes branch July 25, 2019 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants