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

Move unstructured types under meta/v1 to indicate they are typed #38058

Merged
merged 13 commits into from
Dec 11, 2016
Merged

Move unstructured types under meta/v1 to indicate they are typed #38058

merged 13 commits into from
Dec 11, 2016

Conversation

smarterclayton
Copy link
Contributor

Add a few abstractions that make them simpler to use from generic code that does not need accessors. Move OwnerReference to meta/v1 and remote metatypes.go

Part of #37530

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

This change is Reviewable

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. release-note-label-needed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 4, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins kops AWS e2e failed for commit 7b09691a9520a1f5a176d19f89876f9060fba20d. Full PR test history.

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

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2016
@smarterclayton
Copy link
Contributor Author

@stts can you review?

@krousey
Copy link
Contributor

krousey commented Dec 6, 2016

@smarterclayton Apologies. I'm swamped with release duties right now. I probably can't get to this this week.

@smarterclayton
Copy link
Contributor Author

smarterclayton commented Dec 6, 2016 via email

@smarterclayton smarterclayton force-pushed the unstructured branch 2 times, most recently from a2df283 to 9b3e6f7 Compare December 6, 2016 22:42
@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit a2df2830b6e6c5cee27a8f0939aece953ddb7907. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit 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 CRI GCE Node e2e failed for commit a2df2830b6e6c5cee27a8f0939aece953ddb7907. 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 a2df2830b6e6c5cee27a8f0939aece953ddb7907. 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.

@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 6, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit d884893. Full PR test history.

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

sttts
sttts reviewed Dec 8, 2016
View reviewed changes
pkg/apis/meta/v1/unstructured/unstructured.go

func (obj *Unstructured) UnstructuredContent() map[string]interface{} {
if obj.Object == nil {
obj.Object = make(map[string]interface{})
Copy link
Contributor

Choose a reason for hiding this comment

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

intentional to modify it in an accessor?

sttts
sttts reviewed Dec 8, 2016
View reviewed changes
pkg/runtime/interfaces.go
IsUnstructuredObject()
// IsList returns true if this type is a list or matches the list convention - has an array called "items".
IsList() bool
// UnstructuredContent returns a non-nil, mutable map of the contents of this object. Values may be
Copy link
Contributor

Choose a reason for hiding this comment

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

mutable answers my previous question about modifying in an accessor.

sttts
sttts reviewed Dec 8, 2016
View reviewed changes
staging/src/k8s.io/client-go/pkg/api/meta/help.go
@@ -29,9 +29,8 @@ func IsListType(obj runtime.Object) bool {
// if we're a runtime.Unstructured, check to see if we have an `items` key
Copy link
Contributor

Choose a reason for hiding this comment

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

outdated godocs

@caesarxuchao
Copy link
Member

Not from me.

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 10, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 4c0d6e2717b5820e26988a23210b4aff88a8012f. 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.

@smarterclayton
Copy link
Contributor Author 

W1210 13:31:53.002] ERROR: (gcloud.container.clusters.create) Operation [<Operation
W1210 13:31:53.002]  name: u'operation-1481405492745-4a7f0ce4'
W1210 13:31:53.002]  operationType: OperationTypeValueValuesEnum(CREATE_CLUSTER, 1)
W1210 13:31:53.002]  selfLink: u'https://test-container.sandbox.googleapis.com/v1/projects/943316867058/zones/us-central1-f/operations/operation-1481405492745-4a7f0ce4'
W1210 13:31:53.002]  status: StatusValueValuesEnum(DONE, 3)
W1210 13:31:53.003]  statusMessage: u'cluster_api_version not found.'

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

Jenkins CRI GCE e2e failed for commit 4c0d6e2717b5820e26988a23210b4aff88a8012f. Full PR test history.

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

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 10, 2016
@smarterclayton
Copy link
Contributor Author

Green again. Applying label since all comments addressed and more refactorings on the way (any further feedback will apply in the next round).

@smarterclayton smarterclayton added 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. and removed release-note-label-needed labels Dec 10, 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 (batch tested with PRs 38058, 38523)

@k8s-github-robot k8s-github-robot merged commit 543cfa1 into kubernetes:master Dec 11, 2016
@timstclair timstclair mentioned this pull request Mar 23, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

@caesarxuchao caesarxuchao caesarxuchao left review comments

Assignees

@krousey krousey

@caesarxuchao caesarxuchao

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@smarterclayton @k8s-reviewable @k8s-ci-robot @krousey @caesarxuchao @k8s-github-robot @sttts