minio / operator

Simple Kubernetes Operator for MinIO clusters :computer:
https://min.io/docs/minio/kubernetes/upstream/index.html
GNU Affero General Public License v3.0
1.19k stars 450 forks source link

Upgrade sidecar to v6.0.2 #2286

Closed ramondeklein closed 1 month ago

ramondeklein commented 1 month ago

We should keep sidecar in sync with operator. Important changes are missing.

harshavardhana commented 1 month ago

can the release script be updated to update this file?

ramondeklein commented 1 month ago

I think sidecar was separated from operator in v6, so the MinIO pods wouldn't be recreated for each MinIO operator release. The advantage is that if a new operator is released and sidecar doesn't need to be updated, then the MinIO pods don't need to be restarted.

But I think it's mostly a theoretical advantage. Most customers won't set the MinIO image tag in the Tenants resource, so it will probably update the MinIO tag and restart all pods (most operator releases will come with an updated default MinIO version too).

If customers do set a MinIO image tag, then they'll probably update MinIO too, because if they need to test on staging, then you probably also want to test a new MinIO version.

The biggest drawback of separating the sidecar is that the default sidecar version needs to be updated manually in code. We forgot it during our first deployment. So operator v6.02 still uses sidecar v6.0.0 that has the TLS health-check bug :disappointed:

ramondeklein commented 1 month ago

can the release script be updated to update this file?

@pjuarezd I just noticed (when reviewing another PR) that update-versions only updates the sidecar when RELEASE_SIDECAR=true is set. It looks like everything is working correct, but we just have to have a procedure so the person who creates the release knows when to set this flag.

pjuarezd commented 1 month ago

can the release script be updated to update this file?

@pjuarezd I just noticed (when reviewing another PR) that update-versions only updates the sidecar when RELEASE_SIDECAR=true is set. It looks like everything is working correct, but we just have to have a procedure so the person who creates the release knows when to set this flag.

right, we need an automated check to identify if there is code changes in the sidecar/ dir since the last release in release.sh, that way we don’t need to set the flag manually, and avoid the human error.

ramondeklein commented 1 month ago

right, we need an automated check to identify if there is code changes in the sidecar/ dir since the last release in release.sh, that way we don’t need to set the flag manually, and avoid the human error.

The command git log -1 --pretty="format:%H" sidecar should return the last commit hash in sidecar. If that changed from the previous version, then sidecar should be released as well. The biggest challenge is to find the previous release.

We could use git rev-list -n 1 $(git describe --tags --abbrev=0) to determine the commit hash of the last release, but that uses the latest created tag and not the highest release. At this time git describe --tags --abbrev=0 returns v5.0.17, because it was released after v6.0.2. Not sure if sorting works properly, because v6.0.2 is probably "newer" than v6.0.12 when using ordinary text sorting.

pjuarezd commented 1 month ago

what about git tag --sort=v:refname @ramondeklein? you think this do the trick to get the latest release tag?

ramondeklein commented 1 month ago

@pjuarezd That looks fine for this repository, but you want to compare it with the new version too. I think we need to do something like this:

(echo $RELEASE; git tag) | sort --version-sort > tempfile
INDEX=$(grep -n $RELEASE tempfile | sed 's/:.*//' | head -1)
sed -n "$((INDEX - 1))p" tempfile

It will always return the version before the current version. The trick is to include the new version in the sorting too and fetch the previous line. Should always work (except for the initial version).