-
Notifications
You must be signed in to change notification settings - Fork 7
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
Handle byok clusters slightly better #949
Handle byok clusters slightly better #949
Conversation
The usecase here is: * user creates cluster CRD for byok cluster prior to installing agent * CRD should retry until agent is there then reconcile successfully
Visit the preview URL for this PR (updated for commit 59086e6): https://pluralsh-console--pr949-mjg-eng-2060-cluster-ae0hq2hf.web.app (expires Fri, 17 May 2024 15:52:02 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: dd1ffa0705acc6ef7d6db370e6bd6fc390e945ce |
@@ -295,18 +295,20 @@ var _ = Describe("Cluster Controller", Ordered, func() { | |||
Expect(err).NotTo(HaveOccurred()) | |||
Expect(sanitizeClusterStatus(cluster.Status)).To(Equal(sanitizeClusterStatus(v1alpha1.ClusterStatus{ | |||
Status: v1alpha1.Status{ | |||
ID: lo.ToPtr(byokClusterConsoleID), | |||
SHA: lo.ToPtr("CPYLCGRGF2JWFBF3OGRHQQUSBDXW6Y4VMUDQDCQQDEA6G6CAZORQ===="), | |||
// ID: lo.ToPtr(byokClusterConsoleID), |
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.
Could you tell me what the policy is for committing commented code?
Is this for reference or testing?
I think I'm still a bit traumatized from Amazon code reviews 😬
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.
There's no real policy, I commented it out because i wasn't sure whether i needed to revert
The usecase here is:
Test Plan
n/a
Checklist