charts/redpanda: use new sidecar --selector flag #1151
Conversation
chrisseto
requested review from
RafalKorepta,
andrewstucki and
gene-redpanda
as code owners
chrisseto
force-pushed
the
chris/p/sidecar-tests
branch
from
afad9cd to
e3e338c
Compare
| 1 active | ||
| 3 active | ||
| ``` | ||
| And kubectl exec -it "bazquux-0" "rpk redpanda admin brokers list --include-decommissioned | sed -E 's/\s+/ /gm' | cut -d ' ' -f 1,6" will eventually output: |
There was a problem hiding this comment.
cut in the redpanda pod doesn't have the ability to split on \s and I refuse to learn awk, so we get sed.
chrisseto
force-pushed
the
chris/p/sidecar-tests
branch
2 times, most recently
from
1d1f244 to
a23cef1
Compare
|
|
||
| t.Cleanup(func(ctx context.Context) { | ||
| // make sure we remove all finalizers for these or the CRD cleanup will get wedged | ||
| removeAllFinalizers(ctx, t, schema.GroupVersionKind{ |
There was a problem hiding this comment.
seems like we dropped whatever tests were leveraging old versions of the operator and were creating these resources?
There was a problem hiding this comment.
Oh hm, the test that upgrades from v2.4.x is still there. I didn't remove it just because it was passing. However, it should be hanging the entire suite... I'll dig into this.
There was a problem hiding this comment.
I'm not sure if this was actually doing anything as it was installed inside of a vCluster.
I've also opted to remove this test as it's no longer relevant for this instance of the operator (we don't support v2.4.x -> v25.2.x). For the backport, I'll probably keep some of the special casing in the helm chart step 😅
| } | ||
|
|
||
| path := "../charts/redpanda/chart" | ||
| require.NoError(t, helmClient.DependencyBuild(ctx, path)) |
There was a problem hiding this comment.
The current tests are failing due to removal of this line (since you collapsed this helper, you'll probably want to conditionally run this when passed a chart that has a . prefix?):
Given I helm install "redpanda" "../charts/redpanda/chart" with values: # features/helm-chart.feature:5
--
| Error:
| Error Trace: /work/harpoon/internal/testing/helm.go:37
| /work/acceptance/steps/helm.go:37
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:584
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:368
| /work/harpoon/internal/testing/panic.go:119
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:584
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:368
| /work/harpoon/steps.go:58
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:584
| /nix/store/5xvi25nqmbrg58aixp4zgczilfnp7pwg-go-1.24.3/share/go/src/reflect/value.go:368
| /root/go/pkg/mod/github.com/cucumber/[email protected]/internal/models/stepdef.go:182
| /root/go/pkg/mod/github.com/cucumber/[email protected]/suite.go:211
| /root/go/pkg/mod/github.com/cucumber/[email protected]/suite.go:476
| /root/go/pkg/mod/github.com/cucumber/[email protected]/suite.go:532
| Error: Received unexpected error:
| stderr: Error: INSTALLATION FAILED: An error occurred while checking for chart dependencies. You may need to run `helm dependency build` to fetch missing dependencies: found in Chart.yaml, but missing in charts/ directory: console: exit status 1
There was a problem hiding this comment.
I'm considering that to be a build step and I've added a helm dep build call to the task. Some integration tests should be depending on it too and I think it's an accidental race that makes it work at the moment.
chrisseto
force-pushed
the
chris/p/sidecar-tests
branch
6 times, most recently
from
29d121d to
b035f57
Compare
This commit builds upon a previous commit that added the `--selector` flag to the sidecar. It updates the chart to appropriate set this new flag and adds acceptance tests to assert the functionality works as expected. This commit is separate as the v2.x.x branches need to release the updated sidecar before it can be used.
chrisseto
force-pushed
the
chris/p/sidecar-tests
branch
from
b035f57 to
ca8bf33
Compare
|
Note to self and others: The v2.x backports of this PR will require an operator release and version bump PR before the backports can be merged. |
| CLI_ARGS: '--load {{.CLI_ARGS}}' | ||
|
|
||
| build:charts: | ||
| desc: "Run helm dep build for all charts" |
There was a problem hiding this comment.
NIT: It's not all chart only redpanda, but I think this is the place where we should add any additional helm dep invocation.
There was a problem hiding this comment.
😅 redpanda is the only chart any with deps, so technically it is all charts. I named all charts so this gets added onto rather if we ever end up adding new deps. This is exactly what you said.
|
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. |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Based on #1138
afad9cd charts/redpanda: use new sidecar --selector flag
This commit builds upon a previous commit that added the
--selectorflag tothe sidecar. It updates the chart to appropriate set this new flag and adds
acceptance tests to assert the functionality works as expected.
This commit is separate as the v2.x.x branches need to release the updated
sidecar before it can be used.