Fix the retry issue with deployment verification

knative / serving-operator

DEPRECATED: Development continues in https://github.com/knative/operator/

Apache License 2.0

39 stars 48 forks source link

Fix the retry issue with deployment verification #387

Closed houshengbo closed 4 years ago

houshengbo commented 4 years ago

Fixes #

Proposed Changes

This PR fixed the broken retry mechanism, checking whether the deployments reach the desired state. It used to return false, err, which would run the check only once, but we need to return false, nil, till we reach the desired state. This PR made the correct changes for the retry mechanism.

Release Note

knative-prow-robot commented 4 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: houshengbo

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: - ~~[OWNERS](https://github.com/knative/serving-operator/blob/master/OWNERS)~~ [houshengbo] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

aliok commented 4 years ago

@houshengbo Can you give some details about this PR? I think the code changes look fine, but just want to understand what the original problem was.

houshengbo commented 4 years ago

@aliok I added in the comments of the PR.

aliok commented 4 years ago

/lgtm

makes perfect sense :+1: