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.

Sign up for GitHub

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

Jump to bottom

Wait for MinIO to be ready before proceeding with any further tasks. #2172

Merged
merged 1 commit into from
Jun 26, 2024
Merged

Wait for MinIO to be ready before proceeding with any further tasks. #2172

merged 1 commit into from
Jun 26, 2024

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Jun 20, 2024 •
edited
Loading

@cniackz cniackz force-pushed the improve-status-message-1 branch 2 times, most recently from 03691ec to 4347da3 Compare June 20, 2024 20:37
@cniackz cniackz force-pushed the improve-status-message-1 branch from 4347da3 to 8a41a66 Compare June 20, 2024 20:40
ramondeklein
ramondeklein reviewed Jun 21, 2024
View reviewed changes
Copy link
Contributor

@ramondeklein ramondeklein left a comment

Choose a reason for hiding this comment

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

Wouldn't it make more sense that we have a state before provisioning of the user's that is called Waiting for the cluster to become healthy and just run health-checks? When the health-check succeeds, then we start provisioning users.

@cniackz cniackz self-assigned this Jun 21, 2024
@cniackz cniackz added the wip label Jun 21, 2024
@cniackz cniackz force-pushed the improve-status-message-1 branch 2 times, most recently from 9a96f03 to 3acde81 Compare June 21, 2024 17:24
cesnietor
cesnietor reviewed Jun 21, 2024
View reviewed changes
Copy link
Contributor

@cesnietor cesnietor left a comment

Choose a reason for hiding this comment

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

@cniackz could you please add more details on the description like the motivation and how e.g. creating users addresses this? cause the title made me think is was just about a message being logged but is doing more than that.

@cniackz cniackz force-pushed the improve-status-message-1 branch 2 times, most recently from 831f841 to 4636e3d Compare June 21, 2024 17:29
@cniackz cniackz removed the wip label Jun 21, 2024
@cniackz cniackz changed the title [WIP] - Improving status message when it is stuck Improving status message when it is stuck Jun 21, 2024
@cniackz cniackz changed the title Improving status message when it is stuck Create user(s) only when MinIO is actually ready Jun 21, 2024
ramondeklein
ramondeklein requested changes Jun 21, 2024
View reviewed changes
@cniackz cniackz force-pushed the improve-status-message-1 branch from 4636e3d to 635ef39 Compare June 21, 2024 19:12
@cniackz cniackz changed the title Create user(s) only when MinIO is actually ready Let it be Jun 21, 2024
@cniackz cniackz requested a review from ramondeklein June 21, 2024 20:25
ramondeklein
ramondeklein previously approved these changes Jun 21, 2024
View reviewed changes
@ramondeklein
Copy link
Contributor

PS: Please use descripte PR titles

@cniackz cniackz requested a review from kannappanr June 21, 2024 20:49
@cniackz cniackz changed the title Let it be Wait for MinIO to be ready before proceeding with any further tasks. Jun 21, 2024
@cniackz cniackz force-pushed the improve-status-message-1 branch from 635ef39 to 436141f Compare June 21, 2024 20:51
pjuarezd
pjuarezd previously requested changes Jun 22, 2024
View reviewed changes
Copy link
Member

@pjuarezd pjuarezd left a comment

Choose a reason for hiding this comment

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

simpler is better

ramondeklein
ramondeklein previously approved these changes Jun 24, 2024
View reviewed changes
@cniackz cniackz force-pushed the improve-status-message-1 branch from babb272 to 15608d3 Compare June 25, 2024 18:16
@cniackz @shtripat
Wait for MinIO to be ready before proceeding with any further tasks
51663cb 
Apply suggestions from code review

Co-authored-by: Pedro Juarez <pjuarezd@users.noreply.github.com>

Update pkg/controller/main-controller.go

Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
@cniackz cniackz force-pushed the improve-status-message-1 branch from 15608d3 to 51663cb Compare June 25, 2024 20:35
shtripat
shtripat approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

lgtm

@cniackz cniackz dismissed pjuarezd’s stale review June 26, 2024 12:47

because it is done

@cniackz cniackz merged commit d78c5e0 into minio:master Jun 26, 2024
31 checks passed
@cniackz cniackz deleted the improve-status-message-1 branch June 26, 2024 12:47
@pjuarezd pjuarezd mentioned this pull request Aug 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ramondeklein ramondeklein ramondeklein approved these changes

@shtripat shtripat shtripat approved these changes

@cesnietor cesnietor Awaiting requested review from cesnietor

@dvaldivia dvaldivia Awaiting requested review from dvaldivia

@harshavardhana harshavardhana Awaiting requested review from harshavardhana

@kannappanr kannappanr Awaiting requested review from kannappanr

@pjuarezd pjuarezd Awaiting requested review from pjuarezd

Assignees

@cniackz cniackz

Labels
improvement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cniackz @ramondeklein @pjuarezd @shtripat @cesnietor