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.

Sign up for GitHub

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

Jump to bottom

Add a KUBERNETES_NODE_* section to build kubelet/kube-proxy for windows #38919

Merged
merged 1 commit into from
Dec 20, 2016
Merged

Add a KUBERNETES_NODE_* section to build kubelet/kube-proxy for windows #38919

merged 1 commit into from
Dec 20, 2016

Conversation

brendandburns
Copy link
Contributor

@pires @ixdy

Addresses #38785 (I hope)

@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 17, 2016
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Dec 17, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins CRI GCE Node e2e failed for commit f5a3781. Full PR test history.

The magic incantation to run this job again is @k8s-bot cri node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE Node e2e failed for commit f5a3781. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

luxas
luxas reviewed Dec 17, 2016
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

This is a quick fix indeed and might work, but we should do this properly (in time for v1.6, not a cherrypick candidate) and remove kubelet and the proxy from the server targets and refactor the build/release framework to take the node targets into account

hack/lib/golang.sh
linux/arm64
linux/s390x
windows/amd64
windows/386
Copy link
Member

Choose a reason for hiding this comment

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

Do we need 386 here? Are you actually using it?

Copy link
Member

Choose a reason for hiding this comment

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

probably not, though apparently there are still windows/386 users out there. x-ref #28632

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed. Windows 2016 is required for Windows containers, and it's not build for 386 afaik know.

@brendandburns brendandburns assigned luxas and unassigned lavalamp Dec 17, 2016
@brendandburns
Copy link
Contributor Author

@k8s-bot node e2e test this

@brendandburns
Copy link
Contributor Author

@k8s-bot cri node e2e test this

@ixdy
Copy link
Member

ixdy commented Dec 17, 2016

@k8s-bot build this

@brendandburns
Copy link
Contributor Author

@ixdy @luxas I need to add the relevant build-node-tarballs stuff, then I think this should be ready to go.

I'll add it this evening.

@brendandburns
Copy link
Contributor Author

@k8s-bot build this

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 18, 2016
@brendandburns
Copy link
Contributor Author

@k8s-bot build this

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE etcd3 e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce etcd3 e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Cross Build failed for commit d6cbf130fea348848105481518bd5047e3cbe19f. Full PR test history.

The magic incantation to run this job again is @k8s-bot build this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@brendandburns
Copy link
Contributor Author

@k8s-bot build this

@brendandburns
Copy link
Contributor Author

@ixdy @luxas This appears to work correctly and packages a new node tarball.

@luxas I don't want to remove them from the server tarball (yet) because that would likely break numerous release scripts.

Eventually (probably not even in 1.6, but maybe 1.7) we can remove them from the server tarball...

Please take another look.

Thanks!
--brendan

@luxas luxas added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Dec 18, 2016
@luxas
Copy link
Member

luxas commented Dec 18, 2016

LGTM

Yes, I didn't mean it should be done now, but maybe for v1.7 we should remove kubelet and the proxy from the server targets

@ixdy can apply the label if he also thinks it's ok

@brendandburns
Copy link
Contributor Author

Ping to @ixdy for approval on this.

Thanks!

pires
pires approved these changes Dec 20, 2016
View reviewed changes
Copy link
Contributor

@pires pires left a comment

Choose a reason for hiding this comment

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

This works! Thanks @brendandburns.

@brendandburns
Copy link
Contributor Author

@ixdy are you still around? I'm tempted to LGTM this since both @pires and @luxas approved...

@fejta @spxtr for LGTM approval.

@ixdy
Copy link
Member

ixdy commented Dec 20, 2016

yes, sorry, I will take a look today.

ixdy
ixdy approved these changes Dec 20, 2016
View reviewed changes
Copy link
Member

@ixdy ixdy left a comment

Choose a reason for hiding this comment

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

basically LG. you'll probably need to update https://github.com/kubernetes/release/blob/master/lib/releaselib.sh#L480-L491 too.

build/lib/release.sh
cp "${node_bins[@]/#/${LOCAL_OUTPUT_BINPATH}/${platform}/}" \
"${release_stage}/node/bin/"

# TODO: Docker images here
Copy link
Member

Choose a reason for hiding this comment

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

open an issue for this?

@ixdy
Copy link
Member

ixdy commented Dec 20, 2016

/lgtm

I'd like to see an issue filed to possibly remove these from the server tarball, so we reduce duplication.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit e843f77 into kubernetes:master Dec 20, 2016
@pires pires mentioned this pull request Dec 21, 2016
Closed
@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

k8s-github-robot referenced this pull request Feb 1, 2017
Merge pull request #40507 from brendandburns/automated-cherry-pick-of…
9827d09 
…-#38919-upstream-release-1.5

Automatic merge from submit-queue

Add a KUBERNETES_NODE_* section to build kubelet/kube-proxy for windows

@saad-ali @luxas @pires
@ixdy ixdy mentioned this pull request Oct 24, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luxas luxas luxas left review comments

@pires pires pires approved these changes

@ixdy ixdy ixdy approved these changes

Assignees

@luxas luxas

@ixdy ixdy

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@brendandburns @k8s-reviewable @k8s-ci-robot @ixdy @luxas @k8s-github-robot @k8s-cherrypick-bot @pires @lavalamp