feat: nnc client sets ownerref #1133
Conversation
crd/nodenetworkconfig/client.go
Outdated
| _ = v1alpha.AddToScheme(Scheme) | ||
| } | ||
|
|
||
| type OwnerExistsError struct{} |
There was a problem hiding this comment.
can we flip this around to ErrOwnerExists? seems more canonical (https://go.dev/src/os/error.go) even though k/k likes to do "ThingError"
There was a problem hiding this comment.
if we still need it instead of using ctrlutil's similar defined error
There was a problem hiding this comment.
removed, caller can rely on ctrlutil's error with errors.Is/As since we just wrap it
|
|
||
| // SetOwnerRef sets the owner of the NodeNetworkConfig to the given object, using HTTP Patch | ||
| func (c *Client) SetOwnerRef(ctx context.Context, nnc *v1alpha.NodeNetworkConfig, owner metav1.Object) error { | ||
| newNNC := nnc.DeepCopy() |
There was a problem hiding this comment.
I think we should either return the newNNC or mutate the passed nnc, since the patch call will write the resultant object to the ref it gets passed.
There was a problem hiding this comment.
good point, I'll change that
There was a problem hiding this comment.
decided to return instead of mutate since the nnccli.Patch() could fail and we don't want to mutate the caller if we dn't actually make a successful change
crd/nodenetworkconfig/client.go
Outdated
| // SetControllerReference returns an error if the object already has a different owner or | ||
| // if there are issues with the scheme. Since we control the scheme in this package, we can safely | ||
| // assume than any error is caused by an owner already existing | ||
| return &OwnerExistsError{} |
There was a problem hiding this comment.
because I didn't read the docs and didn't know it existed 😆
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
2 participants
Reason for Change:
Adds SetOwnerRef to nnc client which sets the NNC ownership to the given object
Requirements: