-
Notifications
You must be signed in to change notification settings - Fork 61
Stack controller tests #671
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v2 #671 +/- ##
==========================================
+ Coverage 26.17% 33.24% +7.07%
==========================================
Files 25 25
Lines 3947 3949 +2
==========================================
+ Hits 1033 1313 +280
+ Misses 2772 2474 -298
- Partials 142 162 +20 ☔ View full report in Codecov by Sentry. |
ed79d2c
to
2078ab4
Compare
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.
Looks reasonable.
@@ -64,7 +69,7 @@ var _ = BeforeSuite(func() { | |||
// Note that you must have the required binaries setup under the bin directory to perform | |||
// the tests directly. When we run make test it will be setup and used automatically. | |||
BinaryAssetsDirectory: filepath.Join("..", "..", "..", "bin", "k8s", | |||
fmt.Sprintf("1.29.0-%s-%s", runtime.GOOS, runtime.GOARCH)), | |||
fmt.Sprintf("1.28.3-%s-%s", runtime.GOOS, runtime.GOARCH)), |
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.
Why downgrade?
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 suite for the workspace controller uses 1.28.3
and I think the CI script knows that. I guess I used a slightly different scaffolder for the stack controller.
// annotate the out of date stack so that it'll be queued. The value is arbitrary; this | ||
// value gives a bit of context which might be helpful when troubleshooting. | ||
v := fmt.Sprintf("update prerequisite of %s at %s", instance.Name, time.Now().Format(time.RFC3339)) | ||
prereqStack1 := prereqStack.DeepCopy() | ||
a := prereqStack1.GetAnnotations() | ||
if a == nil { | ||
a = map[string]string{} | ||
} | ||
a[shared.ReconcileRequestAnnotation] = v | ||
prereqStack1.SetAnnotations(a) | ||
log.Info("requesting requeue of prerequisite", "name", prereqStack1.Name, "cause", requireErr.Error()) | ||
if err := r.Client.Patch(ctx, prereqStack1, client.MergeFrom(&prereqStack)); err != nil { | ||
// A conflict here may mean the prerequisite has been changed, or it's just been | ||
// run. In any case, requeueing this object means we'll see the new state of the | ||
// world next time around. | ||
return reconcile.Result{}, fmt.Errorf("annotating prerequisite to force requeue: %w", err) | ||
} |
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.
I'm reverting this change and doing a follow-up: #672
Proposed changes
Implements a test suite for the new stack controller.
Closes #654
Related issues (optional)