-
Notifications
You must be signed in to change notification settings - Fork 14
operator: implement Console controller
#1113
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
| require.NoError(t, ctl.Apply(t.Context(), console)) | ||
|
|
||
| // Reconcile the console a few times to ensure determinism. | ||
| for range 3 { |
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.
@RafalKorepta fixed your previous point by moving everything into the loop.
I've also updated the golden files to include the GVK.
66ba6dd to
ca8b378
Compare
|
This PR is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
a2de539 to
e243f95
Compare
|
field indexing issue fixed! |
e243f95 to
cd8cdaa
Compare
|
Documentation and the migration or v2 workaround will be handled in a follow up PR. |
cd8cdaa to
de39bd0
Compare
andrewstucki
left a comment
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.
A few suggestions on DRY'ing this up a bit with some of the ways we're leveraging ClusterSource, only one real necessary change just for correctness if you don't want to leverage those helpers: have to check the Kind/Group of the clusterRef.
de39bd0 to
233b340
Compare
| // Pod eviction happens in a timely fashion. | ||
| `--k3s-arg`, `--kube-apiserver-arg=default-not-ready-toleration-seconds=10@server:*`, | ||
| `--k3s-arg`, `--kube-apiserver-arg=default-unreachable-toleration-seconds=10@server:*`, | ||
| // Disable the traefik Ingress controller. We don't use Ingress for |
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.
This makes a bigger difference than I expected!
|
|
||
| start := time.Now() | ||
| lastLog := start | ||
| interval := intervalFromDeadline(ctx) |
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.
Modified this to make tests run slightly faster as I didn't want to take on the overhead of a fully refactor 😅
f2c9880 to
56b9dfa
Compare
| // InsecureSkipTLSVerify can skip verifying Redpanda self-signed certificate when establish TLS connection to Redpanda | ||
| // +optional | ||
| InsecureSkipTLSVerify bool `json:"insecureSkipTlsVerify"` | ||
| InsecureSkipTLSVerify bool `json:"insecureSkipTlsVerify,omitempty"` |
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.
This was a subtle issue. In the acceptance test for this, I did not specify this key. So when the controller tried to apply the finalizer, it was also trying to add insecureSkipTlsVerify: false and hitting validation errors because we mark this field as immutable. Adding omitempty prevents it from being serialized if it's false.
andrewstucki
left a comment
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.
some nits to maybe fix up, but since you're trying to figure out the v1 testing too, might want to take a look
Move `redpanda.ParseCLIArgs` -> `pkg/chartutils.ParseFlags` to allow sharing across charts. Improve the operator chart's merging of `values.AdditionalCmdFlags` in preparation for new flags.
56b9dfa to
3919413
Compare
|
Hopefully this last push should do it. I did a bit of chart refactoring in 335c0e1. The console controller will be off by default and the chart enables it by default so v1 tests should continue to work untouched. |
9b7faa7 to
0bd4203
Compare
This commit adds a standalone `Console` CR and its controller. Unlike the console stanza in the redpanda chart, this CR deploys console V3. This commit does NOT include a migration from the subchart to the new CR. That will be implemented later.
0bd4203 to
4eb8e23
Compare
This commit adds a standalone
ConsoleCR and its controller. Unlike the console stanza in the redpanda chart, this CR deploys console V3.This commit does NOT include a migration from the subchart to the new CR. That will be implemented later.