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

OVA: Fix phases display for OVA provider creation #670

Merged
merged 6 commits into from
Dec 25, 2023

Conversation

bkhizgiy
Copy link
Member

@bkhizgiy bkhizgiy commented Dec 5, 2023

While the OVA provider pod is being created, the pod status currently displays connectionFailed until the OVA pod is actually up and running, this gives the wrong impression that something went wrong during the OVA provider creation. This fix changes the phases for the OVA provider. Once the provider is added, the status will immediately show staging. If there is a failure during the OVA provider creation process, or if the pod is not running after 10 minutes (consider as a timeout), the status will change to connectionFailed.

@bkhizgiy bkhizgiy requested a review from ahadas as a code owner December 5, 2023 16:31
@bkhizgiy bkhizgiy requested review from liranr23, ahadas and bennyz and removed request for ahadas December 5, 2023 16:32
pkg/controller/provider/validation.go Outdated Show resolved Hide resolved
pkg/controller/provider/validation.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Show resolved Hide resolved
@ahadas
Copy link
Member

ahadas commented Dec 10, 2023

Currently while the OVA provider pod is being created the pod status despised connectionFailed...

@bkhizgiy I've rephrased the description a bit - did you mean "displayed" here?

pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
@bkhizgiy bkhizgiy force-pushed the reconcile branch 4 times, most recently from a6874ad to 4943080 Compare December 12, 2023 15:28
@ahadas ahadas self-requested a review December 24, 2023 10:03
Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

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

much better, thanks @bkhizgiy, commented on few things inside

pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/validation.go Outdated Show resolved Hide resolved
Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

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

minor comments, looks good overall

pkg/controller/provider/controller.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/ova-setup.go Outdated Show resolved Hide resolved
pkg/controller/provider/validation.go Outdated Show resolved Hide resolved
Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

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

@bkhizgiy this PR changed quite a bit, please update its title and description

@ahadas ahadas requested a review from liranr23 December 25, 2023 07:07
@bkhizgiy bkhizgiy changed the title OVA: Add new phases for OVA provider creation OVA: Change phases for OVA provider creation Dec 25, 2023
@bkhizgiy bkhizgiy changed the title OVA: Change phases for OVA provider creation OVA: Fix phases display for OVA provider creation Dec 25, 2023
Copy link

sonarcloud bot commented Dec 25, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.5% Duplication on New Code

See analysis details on SonarCloud

@ahadas ahadas merged commit cb5ea6a into kubev2v:main Dec 25, 2023
10 checks passed
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.

3 participants