charts/deployment: adjust for wrapper script and release v0.23.0

[jecluis]

By using a wrapper script as the container entrypoint, we abstract how arguments to the underlying radosgw binary are passed. Instead, pass arguments known by the script, and let it do what is best.

This pull request has a companion pull request in aquarist-labs/s3gw#758 .

Also, this patchset also releases v0.23.0.

Signed-off-by: Joao Eduardo Luis <[email protected]>

[jecluis]

It's not clear whether the test is failing due to aquarist-labs/s3gw#758 not being merged.

@m-ildefons is there any way we can test the chart with a custom container version, other than modifying the workflow?

[m-ildefons]

In charts/s3gw/ci you'll find yaml files. The test workflow iterates over those files, executing an installation of the chart using each of the yaml files in charts/s3gw/ci in place of the values.yaml. You can define custom images in there, e.g.:

imageRegistry: "quay.io"
imageName: "s3gw/s3gw-joao-custom"
imageTag: "v1.2.3-test"

[giubacc]

I don't think we have tests ensuring rgwCustomArgs: [] and rgwCustomEnvs: [] functionality in charts; given that they are very useful for developing purposes, please @jecluis, have a check to ensure they continue working as expected with this patch.

[jecluis]

@m-ildefons passed with the custom image 🥳 That last commit is to be dropped before merge, and I expect the PR to actually pass anyway once the image changes are merged on aquarist-labs/s3gw .

@giubacc there's no reason for rgwCustomArgs to not work. As for rgwCustomEnv, I'm not sure what to provide to it to actually test it. Insights welcome.

[giubacc]

@giubacc there's no reason for rgwCustomArgs to not work. As for rgwCustomEnv, I'm not sure what to provide to it to actually test it. Insights welcome.

eg, for rgwCustomArgs :

  helm upgrade --wait --install -n s3gw-ha --create-namespace s3gw-ha s3gw/s3gw 
  ...
    --set rgwCustomArgs="{--rgw_relaxed_region_enforcement,1}"

(the rgwCustomEnv has the same pattern usage)

[jecluis]

(the rgwCustomEnv has the same pattern usage)

I understand that, but what key/values would you use with that?

[giubacc]

(the rgwCustomEnv has the same pattern usage)

I understand that, but what key/values would you use with that?

it doesn't matter, you can just put KEY=VALUE and verify that in the s3gw pod you have that env var set and we are good.

[jecluis]

(the rgwCustomEnv has the same pattern usage)

I understand that, but what key/values would you use with that?

it doesn't matter, you can just put KEY=VALUE and verify that in the s3gw pod you have that env var set and we are good.

This update does nothing to environment variables for the pod. Those will still be available the same way they have been.

Are you using that to pass env vars to the pod, not the radosgw binary? Because if so, then having this option named as rgwCustomEnv might not make a lot of sense.

[giubacc]

Are you using that to pass env vars to the pod, not the radosgw binary? Because if so, then having this option named as rgwCustomEnv might not make a lot of sense.

I think this was the original idea: passing arbitrary env vars to the s3gw pod for dev purposes.
Agree that these names should be now be changed in Custom* dropping the rgw part given that we are in the path to hide the underlying radosgw implementation to the user.

[jecluis]

Okay, then that's something to do in a different patch set.

@m-ildefons any other thing comes to mind about this? Otherwise I'll consider it good to merge after merging https://github.com/aquarist-labs/s3gw/pull/758 , dropping the test patch from this PR, and ensuring it is working.

[jecluis]

Well, the tests will fail nonetheless, even after merging aquarist-labs/s3gw#758 because merging that PR doesn't actually push a new image with the latest code to quay -- and we test the chart using whatever is latest on quay. So it will be testing the code against the current image anyway. This workflow seems broken to me.

We will have to wait until we create the next release to merge this.

[m-ildefons]

LGTM