Skip to content

Commit

Permalink
fix(rollout-service): fix 0 version (#2156)
Browse files Browse the repository at this point in the history
The version client was storing the versions seen in the previous call to
`StreamChangedApps` endpoint. Then it would set the versions of this app
to 0 and notify argo cd in case they did not come in the next
`StreamChangedApps` call.
This incorrect behavior stayed from when we got the full overview
instead of just the changed apps and could check every version every
time.

This PR changes the logic so that the version client now stores every
previously seen version and, if a deployment from a changed app is not
sent or sent as nil, deletes missing deployments (set version to 0).

Ref: SRX-0F9ZN3
  • Loading branch information
diogo-nogueira-freiheit authored Dec 3, 2024
1 parent a097aac commit 232e174
Showing 1 changed file with 59 additions and 55 deletions.
114 changes: 59 additions & 55 deletions services/rollout-service/pkg/versions/versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ type key struct {

func (v *versionClient) ConsumeEvents(ctx context.Context, processor VersionEventProcessor, hr *setup.HealthReporter) error {
ctx = auth.WriteUserToGrpcContext(ctx, RolloutServiceUser)
versions := map[key]uint64{}
seenVersions := map[key]uint64{}
environmentGroups := map[key]string{}
teams := map[key]string{}
return hr.Retry(ctx, func() error {
Expand Down Expand Up @@ -226,61 +226,84 @@ func (v *versionClient) ConsumeEvents(ctx context.Context, processor VersionEven
l := logger.FromContext(ctx)

l.Info("overview.get")
seen := make(map[key]uint64, len(versions))

overview := argo.ArgoOverview{
Overview: ov,
AppDetails: nil,
}

for _, appDetailsResponse := range changedApps.ChangedApps {
appName := appDetailsResponse.Application.Name
appsToChange[appName] = appDetailsResponse
v.cache.Add(appName, appDetailsResponse) // Update cache of app details

app := appDetailsResponse.Application
//Go through every deployment and check if we have seen it. If not, Add it to the pool of events
for env, deployment := range appDetailsResponse.Deployments {
dt := deployedAt(deployment)
sc := sourceCommitId(appDetailsResponse.Application.Releases, deployment)
tm := appDetailsResponse.Application.Team

foundEnv := false
var envGroup *api.EnvironmentGroup
for _, currEnvGroup := range overview.Overview.EnvironmentGroups {
for _, currEnv := range currEnvGroup.Environments {
if currEnv.Name == env {
foundEnv = true
envGroup = currEnvGroup
}
}
appSeenVersions := make(map[string]struct{})
for key := range seenVersions {
if key.Application != appName {
continue
}

if !foundEnv {
return fmt.Errorf("getAppDetails returned information regarding a deployment for app %s on env %s, but did not provide any environment group information about this environment", appName, env)
}
appSeenVersions[key.Environment] = struct{}{}
}

l.Info("version.process", zap.String("application", app.Name), zap.String("environment", env), zap.Uint64("version", deployment.Version), zap.Time("deployedAt", dt))
k := key{env, appName}
seen[k] = deployment.Version
environmentGroups[k] = envGroup.EnvironmentGroupName
teams[k] = tm
if versions[k] == deployment.Version {
continue
for _, envGroup := range overview.Overview.EnvironmentGroups {
for _, env := range envGroup.Environments {
argoAppKey := key{Environment: env.Name, Application: appName}
seenVersion, hasVersion := seenVersions[argoAppKey]
deployment, deploymentExists := appDetailsResponse.Deployments[env.Name]

if !deploymentExists || deployment == nil {
continue
}

// Deployment exists, do not delete it
delete(appSeenVersions, env.Name)
if hasVersion && deployment.Version == seenVersion {
continue
}

seenVersions[argoAppKey] = deployment.Version
environmentGroups[argoAppKey] = envGroup.EnvironmentGroupName
teams[argoAppKey] = appDetailsResponse.Application.Team

dt := deployedAt(deployment)
sc := sourceCommitId(appDetailsResponse.Application.Releases, deployment)
l.Info("version.process", zap.String("application", appName), zap.String("environment", env.Name), zap.Uint64("version", deployment.Version), zap.Time("deployedAt", dt))

processor.ProcessKuberpultEvent(ctx, KuberpultEvent{
Application: appName,
Environment: env.Name,
EnvironmentGroup: envGroup.EnvironmentGroupName,
Team: appDetailsResponse.Application.Team,
IsProduction: (envGroup.Priority == api.Priority_PROD || envGroup.Priority == api.Priority_CANARY),
Version: &VersionInfo{
Version: deployment.Version,
SourceCommitId: sc,
DeployedAt: dt,
},
})
}
}
// Delete all environments that we track but we did not see
for missingEnvironment := range appSeenVersions {
deletedArgoAppKey := key{Environment: missingEnvironment, Application: appName}

processor.ProcessKuberpultEvent(ctx, KuberpultEvent{
IsProduction: false,
Application: appName,
Environment: env,
EnvironmentGroup: envGroup.EnvironmentGroupName,
Team: tm,
IsProduction: (envGroup.Priority == api.Priority_PROD || envGroup.Priority == api.Priority_CANARY),
Environment: missingEnvironment,
EnvironmentGroup: environmentGroups[deletedArgoAppKey],
Team: teams[deletedArgoAppKey],
Version: &VersionInfo{
Version: deployment.Version,
SourceCommitId: sc,
DeployedAt: dt,
Version: 0,
SourceCommitId: "",
DeployedAt: time.Time{},
},
})
delete(seenVersions, deletedArgoAppKey)
delete(environmentGroups, deletedArgoAppKey)
delete(teams, deletedArgoAppKey)
}

}

overview.AppDetails = appsToChange
Expand All @@ -290,25 +313,6 @@ func (v *versionClient) ConsumeEvents(ctx context.Context, processor VersionEven
l.Info("version.push")
appsToChange = make(map[string]*api.GetAppDetailsResponse)
}
// Send events with version 0 for deleted applications so that we can react
// to apps getting deleted.
for k := range versions {
if seen[k] == 0 {
processor.ProcessKuberpultEvent(ctx, KuberpultEvent{
IsProduction: false,
Application: k.Application,
Environment: k.Environment,
EnvironmentGroup: environmentGroups[k],
Team: teams[k],
Version: &VersionInfo{
Version: 0,
SourceCommitId: "",
DeployedAt: time.Time{},
},
})
}
}
versions = seen
}
})
}
Expand Down

0 comments on commit 232e174

Please sign in to comment.