-
Notifications
You must be signed in to change notification settings - Fork 29
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
move e2e into its own go module and remove the runtime dependency on controller-runtime #243
Conversation
87ac507
to
f385f6c
Compare
68a8e28
to
50f5bda
Compare
controller-runtime (we still use it in tests).
50f5bda
to
f602d38
Compare
@@ -11,7 +11,7 @@ on: # yamllint disable-line rule:truthy | |||
branches: | |||
- "*" | |||
env: | |||
GO_VERSION: "~1.19" | |||
GO_VERSION: "~1.20" |
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.
move to 1.21 now that its out?
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.
we usually wait a little bit right? I figured we'd do spicedb first (I would want to move to the new stdlb slices/maps libs where possible too)
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.
K
@@ -16,7 +16,7 @@ func (c *Controller) PatchStatus(ctx context.Context, patch *v1alpha1.SpiceDBClu | |||
} | |||
patch.ManagedFields = nil | |||
patch.Status.ObservedGeneration = patch.Generation | |||
data, err := client.Apply.Data(patch) | |||
data, err := json.Marshal(patch) |
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.
any semantic differences between this and the only Apply
stuff?
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.
@@ -62,7 +62,7 @@ func TestEnsureDeploymentHandler(t *testing.T) { | |||
migrationHash: "testtesttesttest", | |||
secretHash: "secret", | |||
existingDeployments: []*appsv1.Deployment{{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{ | |||
metadata.SpiceDBConfigKey: "n549h6bh675h699h697h86h5cchdfq", | |||
metadata.SpiceDBConfigKey: "n59hb4h5f7h5b7h5dchddh79h67dq", |
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.
Does these keys changing imply that existing operators, when updated, will cause all resources to be re-applied?
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.
It does, I didn't look into why this happened specifically, but we did jump from 1.26 client go to 1.28 so I assume it's something in the deployment fields.
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.
LGTM
@@ -11,7 +11,7 @@ on: # yamllint disable-line rule:truthy | |||
branches: | |||
- "*" | |||
env: | |||
GO_VERSION: "~1.19" | |||
GO_VERSION: "~1.20" |
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.
K
See: authzed/controller-idioms#40 for more details