No need to propagate ttlsecondsafterfinished

[chaunceyjiang]

Signed-off-by: chaunceyjiang [email protected]

What type of PR is this? /kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes: Fixes #2197

Special notes for your reviewer: Never delete an object managed by karmada from member clusters. No need to propagate ttlSecondsAfterFinished.

Does this PR introduce a user-facing change?:

No need to propagate `TTLSecondsAfterFinished ` of the `Job`

[karmada-bot]

@XiShanYongYe-Chang: GitHub didn't allow me to request PR reviews from the following users: 1285yvonne, Alphonse-iwnl.

Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @1285yvonne @Alphonse-iwnl @Garrybest @RainbowMango

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chaunceyjiang]

Hi @XiShanYongYe-Chang I updated the description. Please have a look

[Garrybest]

/lgtm

[chaunceyjiang]

@RainbowMango @Alphonse-iwnl Looking forward to your review

[1285yvonne]

/lgtm

Consistent with our fixed code

[karmada-bot]

@1285yvonne: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Consistent with our fixed code

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chaunceyjiang]

the RemoveIrrelevantField is also used by karmadactl promote that should not expect this change.

Yes, I'll research it later

enable ttl-after-finished on Karmada control plane and disable it on member clusters.

No, i think we do not care whether the member clusters is disabled or enabled, because TTLSecondsAfterFinished has not been propagated to the member cluster, even if the member cluster is enabled, it still has no effect

[chaunceyjiang]

the RemoveIrrelevantField is also used by karmadactl promote

https://github.com/karmada-io/karmada/blob/39400eda1a207ae45f631181100c3971075413cc/pkg/resourceinterpreter/defaultinterpreter/prune/prune.go#L15-L17

From this function comment, this function should not be used in karadactl promote ,because the karadactl promote is from the member cluster to the control plane, however karadactl promote has alread used this function.

So can we modify the function signature to support setting a propagation direction？ 😂

type Direction bool

const (
	MemberCluster2karmada Direction = false
	Karmada2MemberCluster Direction = true
)

// RemoveIrrelevantField used to remove fields that generated by kube-apiserver and no need(or can't) propagate to
// member clusters.
func RemoveIrrelevantField(workload *unstructured.Unstructured,direction Direction) error {

[chaunceyjiang]

What do you think of this?

@RainbowMango @XiShanYongYe-Chang

[RainbowMango]

So can we modify the function signature to support setting a propagation direction？

I think RemoveIrrelevantField should care about who and where will call it, if we add a direction it will be coupled with the use case. Having said that, I'm also thinking a similar way, that is introducing an extra parameter to RemoveIrrelevantField to specify the rules of prune.

[chaunceyjiang]

I'm also thinking a similar way, that is introducing an extra parameter to RemoveIrrelevantField to specify the rules of prune

like this? @RainbowMango

// RemoveIrrelevantField used to remove fields that generated by kube-apiserver and no need(or can't) propagate to
// member clusters.
func RemoveIrrelevantField(workload *unstructured.Unstructured, fields ...[]string) error {
	for i := range fields {
               // removes to specify nested field from the obj
		unstructured.RemoveNestedField(workload.Object, fields[i]...)
	}

...

}

[chaunceyjiang]

Can this PR be included in the next release?

@XiShanYongYe-Chang @RainbowMango @Garrybest

[RainbowMango]

Let's try /assign

[RainbowMango]

	   +--------+------------------------------------------+-------------------------------------------------+----------+
	   | number | ttl-after-finished controller in karmada | ttl-after-finished controller in member cluster | conflict |
	   +--------+------------------------------------------+-------------------------------------------------+----------+
	   | 1      | disable                                  | disable                                         | No       |
	   | 2      | disable                                  | enable                                          | Yes      |
	   | 3      | enable                                   | disable                                         | No       |
	   | 4      | enable                                   | enable                                          | Yes      |
	   +--------+------------------------------------------+-------------------------------------------------+----------+

	   In the case of number 2, jobs propagated to member clusters will always be recreated once TTL after finished
	   In the case of number 4, if the member cluster is earlier than karamda's deletion of the completed job, the job of the member cluster will be recreated

The table is great, echo here just for the record, we may need it in docs.

[karmada-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment