Skip to content

Add GetGroupVersionKind and Selector WithPod #1043

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

Closed
savitaashture wants to merge 1 commit into from
Closed

Add GetGroupVersionKind and Selector WithPod #1043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion apis/duck/v1/podspec_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"

"knative.dev/pkg/apis"
"knative.dev/pkg/apis/duck"
Expand All @@ -44,7 +45,8 @@ type WithPod struct {

// WithPodSpec is the shell around the PodSpecable within WithPod.
type WithPodSpec struct {
Template PodSpecable `json:"template,omitempty"`
Selector *metav1.LabelSelector `json:"selector"`
Copy link
Copy Markdown
Contributor

@markusthoemmes markusthoemmes Feb 4, 2020

Choose a reason for hiding this comment

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

This would break compatibility with our own PodSpecables, like Service and Configuration.

Copy link
Copy Markdown
Contributor Author

@savitaashture savitaashture Feb 4, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

If that's the case then do we need to write new one because for controllers selector is mandatory

Copy link
Copy Markdown
Contributor

@markusthoemmes markusthoemmes Feb 4, 2020

Choose a reason for hiding this comment

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

See my comment on the PR. I think we shouldn't actually reserialize from the PodSpecable as that's a lossy conversion (as you noticed).

Instead, we should only override the relevant part of the unstructured that's been updated and part of PodSpecable

Template PodSpecable `json:"template,omitempty"`
}

// Assert that we implement the interfaces necessary to
Expand Down Expand Up @@ -82,6 +84,11 @@ func (*WithPod) GetListType() runtime.Object {
return &WithPodList{}
}

// GetGroupVersionKind returns a GroupVersionKind.
func (*WithPod) GetGroupVersionKind() schema.GroupVersionKind {
Copy link
Copy Markdown
Contributor

@markusthoemmes markusthoemmes Feb 4, 2020

Choose a reason for hiding this comment

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

Not sure if this is the right thing to do here as WithPod doesn't really exist. Should this read the actual GroupVersionKind from the typeMeta?

return SchemeGroupVersion.WithKind("WithPod")
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// WithPodList is a list of WithPod resources
Expand Down
6 changes: 6 additions & 0 deletions apis/duck/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.