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

Refactor/update attempts kubectl apply #751

Merged
merged 29 commits into from
Oct 5, 2020

Conversation

ruionweb
Copy link
Contributor

Addressing context deadline issues associated with the apply command

Increasing the number of attempts on the apply command will not interfere with the syncing of resources I believe, as the resource watcher is initiated after the apply command and begins syncing here. As well any apply failures will be caught here reaching a DeploymentError before a sync.

The Kubectl apply command is also idempotent, should be safe to retry.

@ruionweb ruionweb requested a review from a team as a code owner September 24, 2020 14:15
@timothysmith0609
Copy link
Contributor

This looks good to me except for the linter error in CI. Are you able to investigate that problem?

@ruionweb
Copy link
Contributor Author

Rubocop works fine locally, I'm unsure why it's creating that error

@timothysmith0609
Copy link
Contributor

Ah, ok I'll take a look at fixing that, then. Sorry about that

@ruionweb
Copy link
Contributor Author

cc @Shopify/pipeline

Copy link

@ayatsynych ayatsynych left a comment

Choose a reason for hiding this comment

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

👏 let's see if with 2 reties we still see occurrences of this problem

@ajshepley
Copy link
Contributor

If you rebase with master, you'll get the rubocop changes/fixes.

@ruionweb
Copy link
Contributor Author

ruionweb commented Oct 5, 2020

I rebased it! just waiting for CI pass🤞

@ruionweb ruionweb merged commit 4e47cce into master Oct 5, 2020
@timothysmith0609 timothysmith0609 temporarily deployed to rubygems October 6, 2020 17:34 Inactive
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.

6 participants