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

remove defaulting from conversion path #42914

Merged
merged 1 commit into from
Apr 14, 2017
Merged

remove defaulting from conversion path #42914

merged 1 commit into from
Apr 14, 2017

Conversation

liggitt
Copy link
Member

@liggitt liggitt commented Mar 10, 2017 •
edited
Loading

follow up for #42764

  • remove call to defaulting from conversion path (defaulting is a separate step from conversion)
  • remove non-top-level-object defaulting registration (unused after conversion call is removed)
  • generate missing top-level defaults for some api groups:
    • autoscaling/v2alpha1
    • policy/v1alpha1
    • policy/v1beta1
  • register top-level defaults for some api groups that were missing them:
    • autoscaling/v2alpha1
    • settings/v1alpha1

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

This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/new-api size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 10, 2017
@liggitt liggitt assigned smarterclayton and wojtek-t and unassigned mml Mar 10, 2017
@liggitt liggitt added this to the v1.6 milestone Mar 10, 2017
@liggitt liggitt added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Mar 10, 2017
@ethernetdan ethernetdan added kind/bug Categorizes issue or PR as related to a bug. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. kind/upgrade-test-failure labels Mar 10, 2017
@liggitt
Copy link
Member Author

liggitt commented Mar 10, 2017

Still WIP, tests need updating

@liggitt liggitt changed the title WIP - fix defaulting WIP - remove defaulting from conversion path Mar 11, 2017
@liggitt liggitt removed this from the v1.6 milestone Mar 11, 2017
@liggitt
Copy link
Member Author

liggitt commented Mar 11, 2017

this should still be done, but I'd like more soak time on it, so I'm moving this to 1.7.

I opened #42944 for 1.6 instead, that just fixes the patch defaulting issue

@liggitt liggitt added this to the v1.7 milestone Mar 11, 2017
@wojtek-t
Copy link
Member

wojtek-t commented Apr 5, 2017

@liggitt - do you think we can now proceed with this one?

@liggitt
Copy link
Member Author

liggitt commented Apr 12, 2017

yes, picking this back up now

@liggitt liggitt changed the title WIP - remove defaulting from conversion path remove defaulting from conversion path Apr 12, 2017
@smarterclayton
Copy link
Contributor

Looks ok to me.

@smarterclayton
Copy link
Contributor

/approve

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 12, 2017
@liggitt liggitt removed the kind/bug Categorizes issue or PR as related to a bug. label Apr 12, 2017
@liggitt liggitt removed kind/upgrade-test-failure kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. labels Apr 12, 2017
@wojtek-t
Copy link
Member

The //pkg/api:go_default_xtest tes is failing, which definitely seems related to this PR.

@liggitt
Copy link
Member Author

liggitt commented Apr 13, 2017 

I0412 10:55:55.382] Verifying hack/make-rules/../../hack/verify-generated-protobuf.sh
I0412 10:55:55.753] +++ [0412 10:55:55] Verifying Prerequisites....
I0412 10:55:57.326] +++ [0412 10:55:57] Building Docker image kube-build:build-781be6bdb5-5-v1.7.5-3
I0412 10:56:02.513] +++ [0412 10:56:02] Keeping container amazing_kowalevski
I0412 10:56:02.515] +++ [0412 10:56:02] Keeping container kube-build-data-5a6917f06d-5-v1.7.5-3
I0412 10:56:02.568] +++ [0412 10:56:02] Keeping container amazing_kowalevski
I0412 10:56:02.569] +++ [0412 10:56:02] Keeping container kube-build-data-5a6917f06d-5-v1.7.5-3
I0412 10:56:02.620] +++ [0412 10:56:02] Keeping container amazing_kowalevski
I0412 10:56:02.621] +++ [0412 10:56:02] Keeping container kube-build-data-5a6917f06d-5-v1.7.5-3
I0412 10:56:02.772] +++ [0412 10:56:02] Keeping image kube-build:build-781be6bdb5-5-v1.7.5-3
I0412 10:56:02.774] +++ [0412 10:56:02] Keeping image kube-build:build-5a6917f06d-5-v1.7.5-3
I0412 10:56:02.775] +++ [0412 10:56:02] Keeping image kube-build:build-30d75cdf9d-5-v1.7.5-3
I0412 10:56:02.941] +++ [0412 10:56:02] Creating data container kube-build-data-781be6bdb5-5-v1.7.5-3
I0412 10:56:23.123] +++ [0412 10:56:23] Syncing sources to container
I0412 10:56:23.131] +++ [0412 10:56:23] Stopping any currently running rsyncd container
I0412 10:56:23.242] +++ [0412 10:56:23] Starting rsyncd container
W0412 10:56:28.403] !!! [0412 10:56:28] Could not connect to rsync container. See build/README.md for setting up remote Docker engine.
W0412 10:56:28.406] !!! [0412 10:56:28] Call tree:
W0412 10:56:28.408] !!! [0412 10:56:28]  1: /go/src/k8s.io/kubernetes/hack/../build/../build/common.sh:425 kube::build::sync_to_container(...)
W0412 10:56:28.409] !!! [0412 10:56:28]  2: /go/src/k8s.io/kubernetes/hack/../build/run.sh:29 kube::build::build_image(...)
I0412 10:56:28.510] FAILED   hack/make-rules/../../hack/verify-generated-protobuf.sh	33s

@k8s-bot verify test this

@wojtek-t
Copy link
Member

/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, smarterclayton, wojtek-t

Needs approval from an approver in each of these OWNERS Files:
  • OWNERS [smarterclayton,wojtek-t]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 44424, 44026, 43939, 44386, 42914)

@k8s-github-robot k8s-github-robot merged commit 5ad4940 into kubernetes:master Apr 14, 2017
@liggitt liggitt deleted the fix-defaulting branch April 20, 2017 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

@wojtek-t wojtek-t

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
8 participants
@liggitt @k8s-reviewable @wojtek-t @smarterclayton @k8s-github-robot @mml @ethernetdan @k8s-ci-robot