Skip to content
This repository has been archived by the owner on Dec 1, 2018. It is now read-only.

bump kubernetes to 1.8 #1844

Closed

Conversation

andyxning
Copy link
Contributor

@andyxning andyxning commented Oct 12, 2017

This PR will bump client-go/api/metrics/apimachinery/kubernetes to 1.8.

As node-problem-detector depends on heapster in some pkgs, we need firs to update heapster to support Kubernetes 1.8 first.

Main changes:

  • remove source apiVersion config as Cluster has no APIVersion field for now.
  • remove usage of NodeLegacyHostIP as it has been deprecated.
  • deprecate heapster/metrics/metrics and use metrics/pkg/apis/metrics.
  • udpate metrics from v1alpha1 to v1beta1.

/cc @DirectXMan12 @piosz

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 12, 2017
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 12, 2017
@andyxning andyxning force-pushed the bump_kubernetes_to_1.8 branch 7 times, most recently from 4025be4 to 22697c1 Compare October 12, 2017 15:21
Copy link

@Joseph-Irving Joseph-Irving left a comment

Choose a reason for hiding this comment

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

func (a *Api) Register(container *restful.Container) {
 	ws := new(restful.WebService)
 	ws.Path("/apis/metrics/v1alpha1").
 		Doc("Root endpoint of metrics API").
 		Produces(restful.MIME_JSON)

Should the ws.path now be ws.Path("/apis/metrics.k8s.io/v1beta1") due to the changes to the metrics api in k8s 1.8?

@andyxning
Copy link
Contributor Author

@Joseph-Irving Seems to be what you said.

Actually updating the dependencies is painful. :(

I have tried more than one day to resolve the changes in apimachinery&&api&&metrics&&others.

@andyxning andyxning force-pushed the bump_kubernetes_to_1.8 branch 3 times, most recently from 18b78d5 to c2ab571 Compare October 13, 2017 15:38
@andyxning
Copy link
Contributor Author

@piosz @DirectXMan12 According to the error info in pull-heapster-e2e:

I1013 15:39:23.746] go version
I1013 15:39:23.789] go version go1.7.4 linux/amd64
I1013 15:39:23.789] GOARCH=amd64 CGO_ENABLED=0 go build -ldflags "-w -X k8s.io/heapster/version.HeapsterVersion=v1.5.0-beta.0 -X k8s.io/heapster/version.GitCommit=b75729e" -o heapster k8s.io/heapster/metrics
W1013 15:39:27.133] # k8s.io/heapster/vendor/k8s.io/apimachinery/pkg/util/strategicpatch
W1013 15:39:27.133] vendor/k8s.io/apimachinery/pkg/util/strategicpatch/patch.go:520: undefined: sort.SliceStable
W1013 15:39:39.302] # k8s.io/heapster/vendor/k8s.io/apimachinery/pkg/util/proxy
W1013 15:39:39.303] vendor/k8s.io/apimachinery/pkg/util/proxy/dial.go:78: tlsConfig.Clone undefined (type *tls.Config has no field or method Clone, but does have tls.clone)
I1013 15:39:59.985] Makefile:46: recipe for target 'build' failed
W1013 15:40:00.086] make: *** [build] Error 2

The go version used for pull-heapster-e2e is go1.7.4 which is too old for sort.SliceStable.
I am not sure who to ping to fix or update the golang version used in pull-heapster-e2e.

@Joseph-Irving
Copy link

Joseph-Irving commented Oct 13, 2017

So I did a bit of digging and it seems that the changes to vendor/k8s.io/client-go/tools/cache/reflector.go is causing the server to never start. It used to start a new goroutine but that no longer happens. By having the reflector run in its own routine go reflector.Run(wait.NeverStop) I managed to get it to start however I think it's now starting before populating nodes, pods etc and causing other issues.

@andyxning
Copy link
Contributor Author

ping @loburm @fgrzadkowski Could you please help to fix the pull-heapster-e2e error described in comment above.

@k8s-ci-robot
Copy link
Contributor

@andyxning: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-heapster-e2e 7681f26 link /test pull-heapster-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@fgrzadkowski
Copy link
Contributor

Adding @loburm who is working on some heapster related changes anyway

@fgrzadkowski fgrzadkowski requested review from loburm and removed request for kawych October 16, 2017 15:07
@andyxning
Copy link
Contributor Author

andyxning commented Oct 16, 2017

@fgrzadkowski Could you please help to make the pull-heapster-e2e test PR merged first. We need it to run pull-heapster-e2e test.

It is really hard to update the complicated dependencies. I am not 100 percent sure that it is right to update all the dependency but i have test the PR with our 1.6.3 cluster. And, seems everything is ok.

@fgrzadkowski @loburm Please review the PR for the key points as it is really big to review. :)

@andyxning
Copy link
Contributor Author

/reopen

@andyxning
Copy link
Contributor Author

andyxning commented Oct 17, 2017

@fgrzadkowski Could you please help to reopen this PR as it is accidentally closed by the @k8s-ci-robot . It is quite confused me that i am the PR author and i have no rights to reopen it. :(

@andyxning
Copy link
Contributor Author

/reopen

@andyxning
Copy link
Contributor Author

/cc @loburm

@fgrzadkowski
Copy link
Contributor

Something doesn't work with reopening. @andyxning, I think you deleted your branch?

@andyxning
Copy link
Contributor Author

@fgrzadkowski No. I have not deleted it. It still exists.

Could you help me to reopen this.

@loburm
Copy link
Contributor

loburm commented Oct 17, 2017

I also can't reopen this PR...
Looks like some issue with k8s-ci-robot. It shouldn't close PR in one repo if they have been referenced from the other.

@loburm
Copy link
Contributor

loburm commented Oct 17, 2017

@andyxning don't you mind if I remove your comment with reference to other PR (just a random idea)

@andyxning
Copy link
Contributor Author

i will give it try. If we can not make it i will open another PR to make it.

@andyxning
Copy link
Contributor Author

/reopen

@andyxning
Copy link
Contributor Author

As this PR can not be opened again. I opened another PR #1849 to continue the work on it. :)

@loburm @fgrzadkowski

@loburm
Copy link
Contributor

loburm commented Oct 17, 2017

Thanks for checking. I have created issue that describes this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants