Do not display "Waiting for component to start" twice #4919
Conversation
| @@ -56,6 +57,22 @@ func (a *Adapter) getPod(refresh bool) (*corev1.Pod, error) { | |||
| if refresh || a.pod == nil { | |||
| podSelector := fmt.Sprintf("component=%s", a.ComponentName) | |||
|
|
|||
| // First check if the pod is already in desired state | |||
| c := a.Client.GetKubeClient() | |||
| podList, err := c.KubeClient.CoreV1().Pods(c.Namespace).List(context.TODO(), metav1.ListOptions{ | |||
There was a problem hiding this comment.
I think we can avoid listing the pods and use the condition appsv1.DeploymentAvailable in the WaitAndGetPodWithEvents function to start and stop the spinner
There was a problem hiding this comment.
In fact, I cannot see any use case where this function getPod is really waiting for the pod to be started. @mik-dass do you have any example when this function is waiting?
The only function I can see waiting is WaitForDeploymentRollout
There was a problem hiding this comment.
I changed to start the spinner only when a phase different from the expected one is received
feloy
force-pushed
the
bugfix-4362/waiting-component-start
branch
from
7ab7f3c
to
b131232
Compare
feloy
force-pushed
the
bugfix-4362/waiting-component-start
branch
from
b131232
to
a4b06f4
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
/retest |
| defer spinner.End(false) | ||
| var spinner *log.Status | ||
| defer func() { | ||
| if spinner != nil { |
There was a problem hiding this comment.
In what situation will this check be not satisfied? We're doing var spinner *log.Status right above this so, I'm confused as to when spinner will be nil and this check would be not satisfied.
There was a problem hiding this comment.
In var spinner *log.Status, spinner is a non initialized pointer, so getting its zero value, nil.
The pointer gets affected a value only when receiving a phase different from the expected one.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kadel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
openshift-ci
bot
added
the
lgtm
…er#4919) * Do not display "Waiting for component to start" twice * Changelog
What type of PR is this?
/kind bug
What does this PR do / why we need it:
WaitAndGetPodWithEventsnow waits for an event with a Phase different from the expected one before to start the Spinner. This way, the spinner is only displayed when the pod is not in the expected phase.Which issue(s) this PR fixes:
Fixes #4362
PR acceptance criteria:
Unit test
Integration test
Documentation
Update changelog
I have read the test guidelines
How to test changes / Special notes to the reviewer: