-
Notifications
You must be signed in to change notification settings - Fork 28
feat: send untouched resource info to legacy progress reporter #664
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
b07b69d
d749770
c22e8c4
03f8558
21aa71a
41adc8d
58f648f
9522626
a2a1d05
81e5d77
e526e5a
0695b0e
0a53156
415fc5c
8da6810
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "fmt" | ||
|
|
||
| "github.com/samber/lo" | ||
| "k8s.io/apimachinery/pkg/api/meta" | ||
|
|
||
| kdutil "github.com/werf/kubedog/pkg/trackers/dyntracker/util" | ||
| "github.com/werf/nelm/pkg/legacy/progrep" | ||
|
|
@@ -42,6 +43,17 @@ func (r *LegacyProgressReporter) ReportStatus(opID string, status progrep.Operat | |
| }) | ||
| } | ||
|
|
||
| func (r *LegacyProgressReporter) StartStage(p *Plan, releaseNamespace string, installableResourceInfos []*InstallableResourceInfo, mapper meta.RESTMapper) { | ||
| resolvedNamespaces := buildResolvedNamespaces(p, releaseNamespace, mapper) | ||
|
|
||
| untouchedResolvedNamespaces := make(map[string]string, len(installableResourceInfos)) | ||
| for _, info := range installableResourceInfos { | ||
| untouchedResolvedNamespaces[info.ID()] = resolveNamespace(info.GroupVersionKind, info.Namespace, releaseNamespace, mapper) | ||
| } | ||
|
|
||
| r.startStage(p, resolvedNamespaces, installableResourceInfos, untouchedResolvedNamespaces) | ||
| } | ||
|
|
||
| func (r *LegacyProgressReporter) Stop(ctx context.Context) { | ||
| var report progrep.ProgressReport | ||
|
|
||
|
|
@@ -59,7 +71,7 @@ func (r *LegacyProgressReporter) Stop(ctx context.Context) { | |
| }() | ||
| } | ||
|
|
||
| func (r *LegacyProgressReporter) startStage(p *Plan, resolvedNamespaces map[string]string) { | ||
| func (r *LegacyProgressReporter) startStage(p *Plan, resolvedNamespaces map[string]string, untouched []*InstallableResourceInfo, untouchedResolvedNamespaces map[string]string) { | ||
| r.state.RWTransaction(func(s *progressReporterState) { | ||
| if len(s.ops) > 0 { | ||
| s.frozen = append(s.frozen, buildStageReport(s.ops)) | ||
|
|
@@ -71,6 +83,7 @@ func (r *LegacyProgressReporter) startStage(p *Plan, resolvedNamespaces map[stri | |
| var entries []opEntry | ||
|
|
||
| entryIndex := make(map[string]int) | ||
| seenRefs := make(map[progrep.ObjectRef]struct{}) | ||
|
|
||
| for _, op := range ops { | ||
| if op.Category != OperationCategoryResource && op.Category != OperationCategoryTrack { | ||
|
|
@@ -81,6 +94,7 @@ func (r *LegacyProgressReporter) startStage(p *Plan, resolvedNamespaces map[stri | |
| typ := mapOperationType(op.Type) | ||
| idx := len(entries) | ||
| entryIndex[op.ID()] = idx | ||
| seenRefs[ref] = struct{}{} | ||
|
|
||
| entries = append(entries, opEntry{ | ||
| iteration: int(op.Iteration), | ||
|
|
@@ -90,6 +104,33 @@ func (r *LegacyProgressReporter) startStage(p *Plan, resolvedNamespaces map[stri | |
| }) | ||
| } | ||
|
|
||
| for _, info := range untouched { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A resource with real drift can reach this NoOp/Completed branch. resourceInstallType() returns ResourceInstallTypeNone not only when the live and dry-apply states match, but also when a resource policy (skip-update / skip-create / skip-recreate) forces None despite an actual diff (skippedByPolicy=true). Such a resource produces no plan operation (and mustTrackReadiness is false), so it flows into this loop and is reported as NoOp/Completed. That reads as "unchanged" to the user even though the resource actually differs from the desired state and was only skipped by policy. This is display-only in the legacy progress reporter (deploy behavior is unaffected), but the label is misleading; consider distinguishing "skipped by policy" from "unchanged".
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We return NoOp type, so it is enough |
||
| if info.GetResult == nil { | ||
| continue | ||
| } | ||
|
|
||
| ref := progrep.ObjectRef{ | ||
| GroupVersionKind: info.GroupVersionKind, | ||
| Name: info.Name, | ||
| Namespace: untouchedResolvedNamespaces[info.ID()], | ||
| } | ||
|
|
||
| if _, ok := seenRefs[ref]; ok { | ||
| continue | ||
| } | ||
|
|
||
| seenRefs[ref] = struct{}{} | ||
|
|
||
| entries = append(entries, opEntry{ | ||
| iteration: 0, | ||
| ref: ref, | ||
| status: progrep.OperationStatusCompleted, | ||
| // Untouched resources have no real operation (MustInstall==None); NoOp is a | ||
| // neutral label for an already-present, unchanged resource shown as Completed. | ||
| typ: progrep.OperationTypeNoOp, | ||
| }) | ||
| } | ||
|
|
||
| for _, op := range ops { | ||
| idx, ok := entryIndex[op.ID()] | ||
| if !ok { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The full original installableInfos is passed into the failure plan, so every install resource still present in the cluster is re-emitted as NoOp/Completed in a stage where it is not an operation of that plan. This diverges from the StageReport doc contract ("ALL operations in the plan"). If this is intended (giving the consumer the full picture), consider clarifying the StageReport doc comment to mention these NoOp pseudo-operations.
(generated by pi-pi)