Skip to content
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

Issue #2339 - Controller should compare with latest git revision if app has changed #2543

Merged

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Oct 22, 2019

Checklist:

@alexmt alexmt force-pushed the 2339-use-latest-revision-if-app-changes branch from 50c4818 to 28aad8a Compare October 22, 2019 20:32
@codecov
Copy link

codecov bot commented Oct 22, 2019

Codecov Report

Merging #2543 into master will decrease coverage by 0.05%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2543      +/-   ##
==========================================
- Coverage   38.09%   38.03%   -0.06%     
==========================================
  Files         114      114              
  Lines       15838    15834       -4     
==========================================
- Hits         6033     6023      -10     
- Misses       9021     9032      +11     
+ Partials      784      779       -5
Impacted Files Coverage Δ
controller/appcontroller.go 44.47% <87.5%> (-1.18%) ⬇️
cmd/argocd/commands/cluster.go 3.19% <0%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99426ce...efaaac5. Read the comment docs.

@alexec alexec self-assigned this Oct 22, 2019
@@ -601,6 +601,22 @@ func TestNeedRefreshAppStatus(t *testing.T) {
assert.Equal(t, argoappv1.RefreshTypeHard, refreshType)
assert.Equal(t, CompareWithLatest, compareWith)
}

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you comment what this test is doing please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

app := app.DeepCopy()
ctrl.requestAppRefresh(app.Name, ComparisonWithNothing)
app.Spec.Source.Helm = &argoappv1.ApplicationSourceHelm{
Parameters: []argoappv1.HelmParameter{{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need this parameters?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. added comment which explains the change

@alexmt alexmt force-pushed the 2339-use-latest-revision-if-app-changes branch from 28aad8a to b78cfa0 Compare October 22, 2019 21:47
@alexmt alexmt requested a review from alexec October 22, 2019 21:47
@alexmt alexmt force-pushed the 2339-use-latest-revision-if-app-changes branch from b78cfa0 to efaaac5 Compare October 22, 2019 22:02
@alexmt alexmt merged commit 9c3c2f3 into argoproj:master Oct 22, 2019
@alexmt alexmt deleted the 2339-use-latest-revision-if-app-changes branch October 22, 2019 22:23
alexmt pushed a commit that referenced this pull request Oct 22, 2019
alexmt pushed a commit that referenced this pull request Oct 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants