-
Notifications
You must be signed in to change notification settings - Fork 37
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
New Update Strategy only when workload changes in ManifestWork #133
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 | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,183 @@ | ||||||
# New Update Strategy only when workload changes in ManifestWork | ||||||
|
||||||
## Release Signoff Checklist | ||||||
|
||||||
- [] Enhancement is `provisional` | ||||||
- [] Design details are appropriately documented from clear requirements | ||||||
- [] Test plan is defined | ||||||
- [] Graduation criteria for dev preview, tech preview, GA | ||||||
- [] User-facing documentation is created in [website](https://github.com/open-cluster-management-io/open-cluster-management-io.github.io/) | ||||||
|
||||||
## Summary | ||||||
This proposal is to introduce new options in update stratgey of manifestwork to resolve issues | ||||||
on how to coordinate other actors and work agent when they apply the same resource on the spoke | ||||||
cluster. | ||||||
|
||||||
## Motivation | ||||||
We have introduced several update strategy in `ManifestWork` to resolve resource conflict before: | ||||||
- `CreateOnly` let work-agent to create the resource only and ignore any further change on the | ||||||
resource | ||||||
- `ServerSideApply` utilize the server side apply mechanism in kube-apiserver, so if work-agent | ||||||
and another actor try to operator the same field in a resource, a conflict can be identified. | ||||||
|
||||||
However, some issues are raised indicating the above strategy can still not meet all the requirements | ||||||
- [issue 631](https://github.com/open-cluster-management-io/ocm/issues/631) reports a case that user | ||||||
uses manifestwork to create a configmap, and then another actor on the spoke change the configmap. The | ||||||
user does not want the configmap to be changed back by the work-agent. However, when the configmap resource | ||||||
in the manifestwork is updated, the user wants the configmap to be updated accordingly. | ||||||
- [issue 670](https://github.com/open-cluster-management-io/ocm/issues/670) is a case when the user | ||||||
uses manifestwork to create argocd's `Application` resource. The `Application` resource has an `operation` | ||||||
field which is used to trigger the sync, and the field will be removed by the argocd when the sync is done. | ||||||
User in the manifestwork sets the `operation` field and when argocd removes the field, the user does not want | ||||||
the field to be updated back by the work-agent. | ||||||
- [issue 690](https://github.com/open-cluster-management-io/ocm/issues/690) is the case that user wants | ||||||
to create a deployment using manifestwork, but what HPA on the spoke the control the replicas of the deployment. | ||||||
Hence the user wants the work-agent to ignore the replicas field of the deployment in the manfiestwor if it is | ||||||
set. | ||||||
|
||||||
|
||||||
## Proposal | ||||||
|
||||||
We would like to introduce new options in `Update` and `ServerSideApply` update strategy to resolve the above issue. | ||||||
So user can set the option in the manifestwork, so when another actor on the spoke cluster update the resource. The | ||||||
work-agent will ignore the change and not try to change the resource back. On the other hand, when the resource spec | ||||||
defined in the manifestwork is updated by the user, the work-agent will still update the resource accordingly. In | ||||||
summary, the option is to ignore the change triggered from spoke cluster only. | ||||||
|
||||||
In addition, in `ServerSideApplly` strategy, we would also introduce a new `ignoreDifferences` options similar as what | ||||||
is defined in argoCD (https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/). Such that user can choose to not | ||||||
let work-agent patch certain resource fields. | ||||||
|
||||||
### Design Details | ||||||
|
||||||
#### API change | ||||||
|
||||||
The change will be added into the `updateStratey` field. For `Update` strategy, we will add an option | ||||||
|
||||||
```go | ||||||
type UpdateConfig struct { | ||||||
// OnSpokeChange defines whether the resource should be overriden by the manifestwork it is changed | ||||||
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.
Suggested change
|
||||||
// on the spoke by another actor. | ||||||
// +kubebuilder:default=Override | ||||||
// +kubebuilder:validation:Enum=Override;NoOverride | ||||||
// +kubebuilder:validation:Required | ||||||
// +required | ||||||
OnSpokeChange string `json:"onSpokeChange,omitempty"` | ||||||
} | ||||||
``` | ||||||
|
||||||
For existing `ServerSideApply` strategy, we will add: | ||||||
|
||||||
```go | ||||||
type ServerSideApplyConfig struct { | ||||||
... | ||||||
// IgnoreFields defines a list of json paths in the resource that will not be updated on the spoke. | ||||||
IgnoreFields []string `json:"ignoreFields,omitempty"` | ||||||
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. Does 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. yes, so when something changes in ignoreFields in the mw, the work-agent will not update resource all the time. 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. I still don't quite understand, will IgnoreFields ignore OnSpokeChange? No matter OnSpokeChange is Override or NoOverride, changes on the manifestworks will not updated to the spoke? why do we need the |
||||||
|
||||||
// OnSpokeChange defines whether the resource should be overriden by the manifestwork it is changed | ||||||
// on the spoke by another actor. | ||||||
// +kubebuilder:default=Override | ||||||
// +kubebuilder:validation:Enum=Override;NoOverride | ||||||
// +kubebuilder:validation:Required | ||||||
// +required | ||||||
OnSpokeChange string `json:"onSpokeChange,omitempty"` | ||||||
} | ||||||
``` | ||||||
|
||||||
#### agent implemntation | ||||||
|
||||||
When work-agent identity that the `OnSpokeChange` is set `NoOverride` for a certain resource in the `ManifestWork`, the agent | ||||||
when apply the resource to the spoke cluster will add an annotation with the key `open-cluster-management.io/object-hash`. | ||||||
The value of the annotation is the computed hash of the resource spec in the `ManifestWork`. Later when another actor | ||||||
mikeshng marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
updates the resource, the work-agent will at first check if the object-hash mismatches with the current resource spec | ||||||
in the `ManifestWork`. If it is the same, the resource will not be updated so the change from spoke is ignored. When | ||||||
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. Should we explain the behavior of "Delete" when 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. the resource itself will be deleted, so does the annotation. The annotation will be kept if the deleteOption is Orphan, and in this case, I do not think we need to remove the annotation. |
||||||
the resource in the manifestwork is updated, the annotation will not match which then trigger the work-agent to update. | ||||||
|
||||||
To handle the `IgnoreFields` for `ServerSideApply`, we will remove the fields defined in the `IgnoreFields` and then | ||||||
generates the apply patch. The objec-hash will also be computed considering the `IgnoreFields`. | ||||||
|
||||||
#### examples | ||||||
|
||||||
To resolve issue 642, user can set the manifestwork with `OnSpokeChange` set to `NoOverride` | ||||||
qiujian16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
```yaml | ||||||
apiVersion: work.open-cluster-management.io/v1 | ||||||
kind: ManifestWork | ||||||
metadata: | ||||||
namespace: <target managed cluster> | ||||||
name: hello-work-demo | ||||||
spec: | ||||||
workload: ... | ||||||
manifestConfigs: | ||||||
- resourceIdentifier: | ||||||
resource: secrets | ||||||
namespace: default | ||||||
name: some-secret | ||||||
updateStrategy: | ||||||
type: Update | ||||||
update: | ||||||
onSpokeChange: NoOverride | ||||||
``` | ||||||
|
||||||
To resolve issue 670, user also can do the same for argocd application. | ||||||
qiujian16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
```yaml | ||||||
apiVersion: work.open-cluster-management.io/v1 | ||||||
kind: ManifestWork | ||||||
metadata: | ||||||
namespace: <target managed cluster> | ||||||
name: hello-work-demo | ||||||
spec: | ||||||
workload: ... | ||||||
manifestConfigs: | ||||||
- resourceIdentifier: | ||||||
group: argoproj.io/v1alpha1 | ||||||
resource: application | ||||||
namespace: default | ||||||
name: application1 | ||||||
updateStrategy: | ||||||
type: Update | ||||||
update: | ||||||
onSpokeChange: NoOverride | ||||||
``` | ||||||
|
||||||
To resolve issue 690, user can set like: | ||||||
qiujian16 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
```yaml | ||||||
apiVersion: work.open-cluster-management.io/v1 | ||||||
kind: ManifestWork | ||||||
metadata: | ||||||
namespace: <target managed cluster> | ||||||
name: hello-work-demo | ||||||
spec: | ||||||
workload: ... | ||||||
manifestConfigs: | ||||||
- resourceIdentifier: | ||||||
group: apps/v1 | ||||||
resource: deployment | ||||||
namespace: default | ||||||
name: deploy1 | ||||||
updateStrategy: | ||||||
type: ServerSideApply | ||||||
serverSideApply: | ||||||
ignoreFields: | ||||||
- ".spec.replicas" | ||||||
``` | ||||||
|
||||||
|
||||||
### Test Plan | ||||||
|
||||||
- test on `OnSpokeChange` with `Overrid` and `NonOverride` option | ||||||
- test on `IgnoreFields` with a single field, a full strcutrue and an item in the list. | ||||||
|
||||||
### Graduation Criteria | ||||||
N/A | ||||||
|
||||||
### Upgrade Strategy | ||||||
It will need upgrade on CRD of ManifestWork on hub cluster, and upgrade of work agent on managed cluster. | ||||||
|
||||||
### Version Skew Strategy | ||||||
- The field is optional, and if it is not set, the manifestwork will be updated as is by the work agent with elder version | ||||||
- The elder version work agent will ignore the newly added field field. | ||||||
|
||||||
## Alternatives |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
title: return-resource-status-in-manifestwork | ||
authors: | ||
- "@qiujian16" | ||
reviewers: | ||
- "@deads2k" | ||
- "@elgnay" | ||
- "@zhujian7" | ||
approvers: | ||
- "@elgnay" | ||
creation-date: 2024-11-12 | ||
last-updated: 2021-11-12 | ||
status: provisional | ||
see-also: | ||
- "/enhancements/sig-architecture/47-manifestwork-updatestrategy" |
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.
This will also be able to resolve the zero creationTime in metadata problem, right?
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.
I think it could, but it means we will not revert the change on the spoke by another actor.