-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
✨ Support subresource modification #1922
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,11 +81,20 @@ type Writer interface { | |
// StatusClient knows how to create a client which can update status subresource | ||
// for kubernetes objects. | ||
type StatusClient interface { | ||
Status() StatusWriter | ||
Status() SubResourceWriter | ||
} | ||
|
||
// StatusWriter knows how to update status subresource of a Kubernetes object. | ||
type StatusWriter interface { | ||
// SubResourceClient knows how to create a client which can update subresource | ||
// for kubernetes objects. | ||
type SubResourceClient interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I am wondering is if this should maybe be called something like I am not sure though if it is possible to come up with a sensible non-crud subresource client, since that could basically be anything? Maybe that one would just be a standard http client? Ref #452 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about it it again, maybe we should instead just add more verb(s) once we get there? |
||
SubResource(subResource string) SubResourceWriter | ||
} | ||
|
||
// StatusWriter is kept for backward compatibility. | ||
type StatusWriter = SubResourceWriter | ||
|
||
// SubResourceWriter knows how to update subresource of a Kubernetes object. | ||
type SubResourceWriter interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may also have to define a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could probably in the future replace the currrent SubResourceWriter interface with a SubResourceClient and then add read functionality in there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or we could have both SubResourceReader and SubResourceWriter, like the Reader and Writer? And also the SubResourceWriter needs more methods other than Update and Patch, for example the subresource pods/binding has to be POST, which is actually Create. |
||
// Update updates the fields corresponding to the status subresource for the | ||
// given obj. obj must be a struct pointer so that obj can be updated | ||
// with the content returned by the Server. | ||
|
@@ -102,6 +111,7 @@ type Client interface { | |
Reader | ||
Writer | ||
StatusClient | ||
SubResourceClient | ||
|
||
// Scheme returns the scheme this client is using. | ||
Scheme() *runtime.Scheme | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ import ( | |
|
||
var _ Reader = &typedClient{} | ||
var _ Writer = &typedClient{} | ||
var _ StatusWriter = &typedClient{} | ||
var _ SubResourceWriter = &typedClient{} | ||
|
||
// client is a client.Client that reads and writes directly from/to an API server. It lazily initializes | ||
// new clients at the time they are used, and caches the client. | ||
|
@@ -159,8 +159,8 @@ func (c *typedClient) List(ctx context.Context, obj ObjectList, opts ...ListOpti | |
Into(obj) | ||
} | ||
|
||
// UpdateStatus used by StatusWriter to write status. | ||
func (c *typedClient) UpdateStatus(ctx context.Context, obj Object, opts ...UpdateOption) error { | ||
// UpdateSubResource used by SubResourceWriter to write status. | ||
func (c *typedClient) UpdateSubResource(ctx context.Context, obj Object, subResource string, opts ...UpdateOption) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO, one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, we definitely need that, apart from Scale there is also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SGTM, or we can have two objects in the UpdateSubResource parameters like If the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Due to the fact that it is not always mandatory I'd prefer to make it part of the opts and we can then provide a convenient wrapper, so that you can do something like:
|
||
o, err := c.cache.getObjMeta(obj) | ||
if err != nil { | ||
return err | ||
|
@@ -173,15 +173,15 @@ func (c *typedClient) UpdateStatus(ctx context.Context, obj Object, opts ...Upda | |
NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). | ||
Resource(o.resource()). | ||
Name(o.GetName()). | ||
SubResource("status"). | ||
SubResource(subResource). | ||
Body(obj). | ||
VersionedParams((&UpdateOptions{}).ApplyOptions(opts).AsUpdateOptions(), c.paramCodec). | ||
Do(ctx). | ||
Into(obj) | ||
} | ||
|
||
// PatchStatus used by StatusWriter to write status. | ||
func (c *typedClient) PatchStatus(ctx context.Context, obj Object, patch Patch, opts ...PatchOption) error { | ||
// PatchSubResource used by SubResourceWriter to write subresource. | ||
func (c *typedClient) PatchSubResource(ctx context.Context, obj Object, subResource string, patch Patch, opts ...PatchOption) error { | ||
o, err := c.cache.getObjMeta(obj) | ||
if err != nil { | ||
return err | ||
|
@@ -197,7 +197,7 @@ func (c *typedClient) PatchStatus(ctx context.Context, obj Object, patch Patch, | |
NamespaceIfScoped(o.GetNamespace(), o.isNamespaced()). | ||
Resource(o.resource()). | ||
Name(o.GetName()). | ||
SubResource("status"). | ||
SubResource(subResource). | ||
Body(data). | ||
VersionedParams(patchOpts.ApplyOptions(opts).AsPatchOptions(), c.paramCodec). | ||
Do(ctx). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! Thanks for the suggestion. I've applied it here and to other similar places as well.