gitops-engine icon indicating copy to clipboard operation
gitops-engine copied to clipboard

feat: Extend sync-options annotation with Force=true (#525)

Open n9 opened this issue 1 year ago • 12 comments

Implementation for #525, #414, https://github.com/argoproj/argo-cd/issues/5882

n9 avatar May 30 '23 08:05 n9

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell C 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar May 30 '23 08:05 sonarqubecloud[bot]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (187312f) 54.67% compared to head (4b3b25d) 54.68%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #526   +/-   ##
=======================================
  Coverage   54.67%   54.68%           
=======================================
  Files          41       41           
  Lines        4635     4636    +1     
=======================================
+ Hits         2534     2535    +1     
  Misses       1905     1905           
  Partials      196      196           
Files Changed Coverage Δ
pkg/sync/common/types.go 54.16% <ø> (ø)
pkg/sync/sync_context.go 74.10% <100.00%> (+0.02%) :arrow_up:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar May 30 '23 08:05 codecov[bot]

@crenshaw-dev How would you like to refactor the method in order to reduce the Cognitive Complexity?

n9 avatar May 30 '23 08:05 n9

@crenshaw-dev I have found: https://github.com/argoproj/gitops-engine/blob/c0ffe8428a23011ae9f6d0d3d01f9bf6200a4339/pkg/sync/sync_context_test.go#L755

But it seems to me that it just tests that the correct method is used.

How the "force" flag is tested? (The "force" mode already exists in ArgoCD. This PR adds just the ability to specify it via an annotation.)

n9 avatar Jun 05 '23 07:06 n9

Any updates on this?

sidharthramesh avatar Jun 18 '23 10:06 sidharthramesh

@n9 good question... I'm not sure how resourceOps.GetLastResourceCommand(kube.GetResourceKey(tc.target) works, but basically I'm looking for a way to verify that, when the Force=true option is specified, the kubectl action is actually performed with --force enabled.

crenshaw-dev avatar Aug 04 '23 21:08 crenshaw-dev

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Aug 17 '23 14:08 sonarqubecloud[bot]

@crenshaw-dev resourceOps.GetLastResourceCommand does not contain flags (such as --force) only the raw command (such as apply or replace).

n9 avatar Aug 17 '23 14:08 n9

@n9 I believe the mock ApplyResource function could be modified to store that flag information.

func (r *MockResourceOps) ApplyResource(ctx context.Context, obj *unstructured.Unstructured, dryRunStrategy cmdutil.DryRunStrategy, force, validate, serverSideApply bool, manager string) (string, error) {
	r.SetLastValidate(validate)
	r.SetLastServerSideApply(serverSideApply)
	r.SetLastServerSideApplyManager(manager)
	r.SetLastResourceCommand(kube.GetResourceKey(obj), "apply")
	command, ok := r.Commands[obj.GetName()]
	if !ok {
		return "", nil
	}

	return command.Output, command.Err
}

crenshaw-dev avatar Aug 21 '23 14:08 crenshaw-dev

Hey @n9 @crenshaw-dev, any updates on testing this and proceeding forward? IIUC you want to see if force flag is visible via ArgoCD logs when sync happens (application-controller pod)?

mcoklica1 avatar Dec 05 '23 10:12 mcoklica1

Unfortunately, I have to work on other things. Feel free to complete this PR.

n9 avatar Dec 05 '23 10:12 n9

@crenshaw-dev Implemented unit test. Could you please take a look at this PR when you have some time? Thank you.

kkk777-7 avatar Dec 09 '23 09:12 kkk777-7

This seems to be obsolete now that https://github.com/argoproj/gitops-engine/pull/560 has been merged, I think.

ChristianCiach avatar Apr 18 '24 21:04 ChristianCiach

Hi!

First of all, thank you for the change!

May I ask how (and when) will this be integrated into Argo CD? All I see for gitops-engine is that the latest release is 2 years old here https://pkg.go.dev/github.com/argoproj/gitops-engine?tab=versions So I don't quite get the release management of this component.

ktzsolt avatar May 10 '24 14:05 ktzsolt