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

EVEREST-1814 add creating state #653

Open
wants to merge 1 commit into
base: EVEREST-1711-ui-remove-final-confirmation-step-from-the-wizard
Choose a base branch
from

Conversation

oksana-grishchenko
Copy link
Contributor

@oksana-grishchenko oksana-grishchenko commented Feb 5, 2025

Add "Creating" DB status

Problem:
EVEREST-1814

Problem:
The DB can be in an ‘unknown’ state for a couple of seconds and we didn't want to show that to the user.

Solution:

The upstream operators have the different statuses when a cluster is being created:

  • pxc - "unknown"
  • psmdb - ""
  • pg does not have any empty status.

So the upstream statuses are different although Everest maps the DB status to the upstream cluster status 1:1. There is no point so far to create a separate mapping for each upstream operator only because we want to unify AppStateCreating, so in this PR the Everest operator unifies the "" and "unknown" statuses into the "creating" status.

Related PRs:

CHECKLIST

Helm chart

  • Is the helm chart updated with the new changes? (if applicable)

Jira

  • Is the Jira ticket created and referenced properly?

Tests

  • Is an Integration test/test case added for the new feature/change?
  • Are unit tests added where appropriate?

@oksana-grishchenko oksana-grishchenko added the test/e2e Run full E2E test suite label Feb 5, 2025
@oksana-grishchenko oksana-grishchenko marked this pull request as ready for review February 5, 2025 14:47
@oksana-grishchenko oksana-grishchenko requested a review from a team as a code owner February 5, 2025 14:47
@@ -33,6 +33,8 @@ var (
const (
// AppStateUnknown is an unknown state.
AppStateUnknown AppState = "unknown"
Copy link
Member

Choose a reason for hiding this comment

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

should we remove this since it is not used anywhere now?

Copy link

Choose a reason for hiding this comment

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

in addition - is AppStateNew used somehow? and shall it be replaced with AppStateCreating as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way we map statuses now is everestv1alpha1.AppState(upstream.Status.State) which means we still need both

AppStateUnknown AppState = "unknown"
AppStateNew AppState = ""

because pxc has "unknown", psmdb has "", pg does not have any empty status.

As an alternative we could remove the statuses^ and have a switch mapping in each provider separately. I thought it's not really needed right now because the other statuses are mapping 1:1. Do you think it would be better to have the mapping for each upstream operator? I agree it's a bit confusing to have the statuses that are used only internally, but not sure which option would be better here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test/e2e Run full E2E test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants