Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Add CRD waiter for migration job and webhook server #2719

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Oct 3, 2019

Description

  • Add CRD waiter for migration job and webhook server

It may take some time before Catalog CRDs registration shows up in main API Server.
Because of that the CRD waiter is executed for each binary. Additionally, we added
retries when restoring Service Catalog resources during the migration process.

See discussion in this thread for more info: https://kubernetes.slack.com/archives/C232SF3TK/p1570107013007500

Related issue:

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mszostok

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 3, 2019
@mszostok
Copy link
Contributor Author

mszostok commented Oct 3, 2019

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2019
@mszostok mszostok force-pushed the fix-upgrade-job branch 2 times, most recently from 5ca08e0 to 13da337 Compare October 3, 2019 15:08
It may take some time before Catalog CRDs registration shows up in main API Server.
Because of that the CRD waiter is executed for each binary. Additionally, we added
retries when restoring Service Catalog resources during the migration process.
@mszostok
Copy link
Contributor Author

mszostok commented Oct 3, 2019

@jberkhahn PTAL

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 3, 2019
// RetryOnError allows the caller to retry fn in case the error returned by fn is retriable
// according to the provided function. backoff defines the maximum retries and the wait
// interval between two retries.
func RetryOnError(backoff wait.Backoff, fn func() error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I could have sworn we had a util function for this somewhere, but I can't seem to find it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tomorrow, I will double recheck that. Basically, this func was inspired by the client-go retry policy but maybe it is already somewhere in our code.

btw. thanks for the review:)

@jberkhahn
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 3, 2019
@k8s-ci-robot k8s-ci-robot merged commit 33771e8 into kubernetes-retired:master Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants