Skip to content

Commit

Permalink
Merge branch 'argoproj:master' into dev
Browse files Browse the repository at this point in the history
  • Loading branch information
rahulbollisetty authored Oct 18, 2024
2 parents c9e1edd + ec499bb commit 298726a
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 27 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ WORKDIR /home/argocd
####################################################################################################
# Argo CD UI stage
####################################################################################################
FROM --platform=$BUILDPLATFORM docker.io/library/node:22.9.0@sha256:69e667a79aa41ec0db50bc452a60e705ca16f35285eaf037ebe627a65a5cdf52 AS argocd-ui
FROM --platform=$BUILDPLATFORM docker.io/library/node:23.0.0@sha256:e643c0b70dca9704dff42e12b17f5b719dbe4f95e6392fc2dfa0c5f02ea8044d AS argocd-ui

WORKDIR /src
COPY ["ui/package.json", "ui/yarn.lock", "./"]
Expand Down
23 changes: 20 additions & 3 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -936,9 +936,7 @@ func useDiffCache(noCache bool, manifestInfos []*apiclient.ManifestResponse, sou
return false
}

currentSpec := app.BuildComparedToStatus()
specChanged := !reflect.DeepEqual(app.Status.Sync.ComparedTo, currentSpec)
if specChanged {
if !specEqualsCompareTo(app.Spec, app.Status.Sync.ComparedTo) {
log.WithField("useDiffCache", "false").Debug("specChanged")
return false
}
Expand All @@ -947,6 +945,25 @@ func useDiffCache(noCache bool, manifestInfos []*apiclient.ManifestResponse, sou
return true
}

// specEqualsCompareTo compares the application spec to the comparedTo status. It normalizes the destination to match
// the comparedTo destination before comparing. It does not mutate the original spec or comparedTo.
func specEqualsCompareTo(spec v1alpha1.ApplicationSpec, comparedTo v1alpha1.ComparedTo) bool {
// Make a copy to be sure we don't mutate the original.
specCopy := spec.DeepCopy()
currentSpec := specCopy.BuildComparedToStatus()

// The spec might have been augmented to include both server and name, so change it to match the comparedTo before
// comparing.
if comparedTo.Destination.Server == "" {
currentSpec.Destination.Server = ""
}
if comparedTo.Destination.Name == "" {
currentSpec.Destination.Name = ""
}

return reflect.DeepEqual(comparedTo, currentSpec)
}

func (m *appStateManager) persistRevisionHistory(
app *v1alpha1.Application,
revision string,
Expand Down
40 changes: 40 additions & 0 deletions controller/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,8 @@ func TestIsLiveResourceManaged(t *testing.T) {
}

func TestUseDiffCache(t *testing.T) {
t.Parallel()

type fixture struct {
testName string
noCache bool
Expand Down Expand Up @@ -1691,6 +1693,44 @@ func TestUseDiffCache(t *testing.T) {
expectedUseCache: false,
serverSideDiff: false,
},
{
// There are code paths that modify the ApplicationSpec and augment the destination field with both the
// destination server and name. Since both fields are populated in the app spec but not in the comparedTo,
// we need to make sure we correctly compare the fields and don't miss the cache.
testName: "will return true if the app spec destination contains both server and name, but otherwise matches comparedTo",
noCache: false,
manifestInfos: manifestInfos("rev1"),
sources: sources(),
app: app("httpbin", "rev1", false, &argoappv1.Application{
Spec: argoappv1.ApplicationSpec{
Destination: argoappv1.ApplicationDestination{
Server: "https://kubernetes.default.svc",
Name: "httpbin",
Namespace: "httpbin",
},
},
Status: argoappv1.ApplicationStatus{
Resources: []argoappv1.ResourceStatus{},
Sync: argoappv1.SyncStatus{
Status: argoappv1.SyncStatusCodeSynced,
ComparedTo: argoappv1.ComparedTo{
Destination: argoappv1.ApplicationDestination{
Server: "https://kubernetes.default.svc",
Namespace: "httpbin",
},
},
Revision: "rev1",
},
ReconciledAt: &metav1.Time{
Time: time.Now().Add(-time.Hour),
},
},
}),
manifestRevisions: []string{"rev1"},
statusRefreshTimeout: time.Hour * 24,
expectedUseCache: true,
serverSideDiff: true,
},
}

for _, tc := range cases {
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ require (
github.com/patrickmn/go-cache v2.1.0+incompatible
github.com/prometheus/client_golang v1.20.5
github.com/r3labs/diff v1.1.0
github.com/redis/go-redis/v9 v9.6.2
github.com/redis/go-redis/v9 v9.7.0
github.com/robfig/cron/v3 v3.0.1
github.com/sirupsen/logrus v1.9.3
github.com/skratchdot/open-golang v0.0.0-20160302144031-75fb7ed4208c
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,8 @@ github.com/r3labs/diff v1.1.0 h1:V53xhrbTHrWFWq3gI4b94AjgEJOerO1+1l0xyHOBi8M=
github.com/r3labs/diff v1.1.0/go.mod h1:7WjXasNzi0vJetRcB/RqNl5dlIsmXcTTLmF5IoH6Xig=
github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/redis/go-redis/v9 v9.0.0-rc.4/go.mod h1:Vo3EsyWnicKnSKCA7HhgnvnyA74wOA69Cd2Meli5mmA=
github.com/redis/go-redis/v9 v9.6.2 h1:w0uvkRbc9KpgD98zcvo5IrVUsn0lXpRMuhNgiHDJzdk=
github.com/redis/go-redis/v9 v9.6.2/go.mod h1:0C0c6ycQsdpVNQpxb1njEQIqkx5UcsM8FJCQLgE9+RA=
github.com/redis/go-redis/v9 v9.7.0 h1:HhLSs+B6O021gwzl+locl0zEDnyNkxMtf/Z3NNBMa9E=
github.com/redis/go-redis/v9 v9.7.0/go.mod h1:f6zhXITC7JUJIlPEiBOTXxJgPLdZcA93GewI7inzyWw=
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
Expand Down
12 changes: 6 additions & 6 deletions pkg/apis/application/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1056,15 +1056,15 @@ func (a *ApplicationStatus) GetRevisions() []string {

// BuildComparedToStatus will build a ComparedTo object based on the current
// Application state.
func (app *Application) BuildComparedToStatus() ComparedTo {
func (spec *ApplicationSpec) BuildComparedToStatus() ComparedTo {
ct := ComparedTo{
Destination: app.Spec.Destination,
IgnoreDifferences: app.Spec.IgnoreDifferences,
Destination: spec.Destination,
IgnoreDifferences: spec.IgnoreDifferences,
}
if app.Spec.HasMultipleSources() {
ct.Sources = app.Spec.Sources
if spec.HasMultipleSources() {
ct.Sources = spec.Sources
} else {
ct.Source = app.Spec.GetSource()
ct.Source = spec.GetSource()
}
return ct
}
Expand Down
2 changes: 1 addition & 1 deletion ui-test/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM docker.io/library/node:22.9.0@sha256:69e667a79aa41ec0db50bc452a60e705ca16f35285eaf037ebe627a65a5cdf52 as node
FROM docker.io/library/node:23.0.0@sha256:e643c0b70dca9704dff42e12b17f5b719dbe4f95e6392fc2dfa0c5f02ea8044d as node

RUN apt-get update && apt-get install --no-install-recommends -y \
software-properties-common
Expand Down
4 changes: 2 additions & 2 deletions ui-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
"dependencies": {
"@types/selenium-webdriver": "^4.1.26",
"assert": "^2.1.0",
"chromedriver": "^129.0.4",
"chromedriver": "^130.0.0",
"selenium-webdriver": "^4.25.0"
},
"devDependencies": {
"@types/mocha": "^10.0.9",
"@types/node": "^22.7.5",
"@types/node": "^22.7.6",
"dotenv": "^16.4.5",
"mocha": "^10.7.3",
"prettier": "^2.8.8",
Expand Down
16 changes: 8 additions & 8 deletions ui-test/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,10 @@
resolved "https://registry.yarnpkg.com/@types/mocha/-/mocha-10.0.9.tgz#101e9da88d2c02e5ac8952982c23b224524d662a"
integrity sha512-sicdRoWtYevwxjOHNMPTl3vSfJM6oyW8o1wXeI7uww6b6xHg8eBznQDNSGBCDJmsE8UMxP05JgZRtsKbTqt//Q==

"@types/node@*", "@types/node@^22.7.5":
version "22.7.5"
resolved "https://registry.yarnpkg.com/@types/node/-/node-22.7.5.tgz#cfde981727a7ab3611a481510b473ae54442b92b"
integrity sha512-jML7s2NAzMWc//QSJ1a3prpk78cOPchGvXJsC3C6R6PSMoooztvRVQEz89gmBTBY1SPMaqo5teB4uNHPdetShQ==
"@types/node@*", "@types/node@^22.7.6":
version "22.7.6"
resolved "https://registry.yarnpkg.com/@types/node/-/node-22.7.6.tgz#3ec3e2b071e136cd11093c19128405e1d1f92f33"
integrity sha512-/d7Rnj0/ExXDMcioS78/kf1lMzYk4BZV8MZGTBKzTGZ6/406ukkbYlIsZmMPhcR5KlkunDHQLrtAVmSq7r+mSw==
dependencies:
undici-types "~6.19.2"

Expand Down Expand Up @@ -267,10 +267,10 @@ chokidar@^3.5.3:
optionalDependencies:
fsevents "~2.3.2"

chromedriver@^129.0.4:
version "129.0.4"
resolved "https://registry.yarnpkg.com/chromedriver/-/chromedriver-129.0.4.tgz#64e598061b74f0f65fae605242327af84ebbfd28"
integrity sha512-j5I55cQwodFJUaYa1tWUmj2ss9KcPRBWmUa5Qonq3X8kqv2ASPyTboFYb4YB/YLztkYTUUw2E43txXw0wYzT/A==
chromedriver@^130.0.0:
version "130.0.0"
resolved "https://registry.yarnpkg.com/chromedriver/-/chromedriver-130.0.0.tgz#a6251ebfc4aeaca9bc2f22cddc4fcf7a105c04b8"
integrity sha512-1g1eMoKF22Uh6l8DTFOPvWLovINPrkAMw7yDHlF6Rx+4W4JI9aGdCZ2Cx7c181hUgALU1oSKGH3uKNryYM5DaQ==
dependencies:
"@testim/chrome-version" "^1.1.4"
axios "^1.7.4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -920,19 +920,19 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
iconClassName: 'fa fa-info-circle',
title: <ActionMenuItem actionLabel='Details' />,
action: () => this.selectNode(fullName),
disabled: !app.spec.source
disabled: !app.spec.source && (!app.spec.sources || app.spec.sources.length === 0)
},
{
iconClassName: 'fa fa-file-medical',
title: <ActionMenuItem actionLabel='Diff' />,
action: () => this.selectNode(fullName, 0, 'diff'),
disabled: app.status.sync.status === appModels.SyncStatuses.Synced || !app.spec.source
disabled: app.status.sync.status === appModels.SyncStatuses.Synced || (!app.spec.source && (!app.spec.sources || app.spec.sources.length === 0))
},
{
iconClassName: 'fa fa-sync',
title: <ActionMenuItem actionLabel='Sync' />,
action: () => AppUtils.showDeploy('all', null, this.appContext.apis),
disabled: !app.spec.source
disabled: !app.spec.source && (!app.spec.sources || app.spec.sources.length === 0)
},
{
iconClassName: 'fa fa-info-circle',
Expand Down

0 comments on commit 298726a

Please sign in to comment.