Skip to content
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

Updates for two October releases #1065

Merged
merged 6 commits into from
Nov 15, 2023
Merged

Updates for two October releases #1065

merged 6 commits into from
Nov 15, 2023

Conversation

djwfyi
Copy link
Collaborator

@djwfyi djwfyi commented Nov 8, 2023

@djwfyi djwfyi requested review from feorlen and ravindk89 November 8, 2023 16:52
@djwfyi djwfyi self-assigned this Nov 8, 2023

MinIO supports three types of :kube-docs:`secrets in Kubernetes <concepts/configuration/secret/#secret-types>`.

#. opaque
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are opaque and tls here keywords, of a sort that might get inline code ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are specific types in the linked K8s docs list.

Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM mod clarity on how much we can imply about support prior to 5.0.10


Running on Kubernetes 1.21 or later.

For the best support of *tls* or *cert-manager* secrets, upgrade to Operator version 5.0.10 or later.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cesnietor if support for these two types are effectively broken pre 5.0.10, should we just say "support added" and go that way?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry Cesar! Stupid github autoselect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, @ravindk89, currently, cert-manager can function as expected when we provide the specified name for the certificate. Additionally, there is a pending PR that, once merged, will prevent the unnecessary request for this secret to be mounted. Therefore, I anticipate no issues after version 5.0.10.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@feorlen feorlen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending Eng input

@djwfyi djwfyi requested a review from cniackz November 8, 2023 21:42
Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @ravind. We should simply mention 'support added' without specifying any particular version, as that information would likely become outdated soon. Apart from that, the PR appears satisfactory to me.

@djwfyi djwfyi merged commit 09ce8ed into main Nov 15, 2023
@djwfyi djwfyi deleted the server-2023-10-14 branch November 15, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RELEASE] mc RELEASE.2023-10-24T05-18-28Z [RELEASE] Updates for three releases in mid-October
5 participants