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

Return error from createContainerURL if storage settings are not configured #156

Merged
merged 1 commit into from
Mar 7, 2022

Conversation

peterbom
Copy link
Contributor

@peterbom peterbom commented Mar 6, 2022

This fixes a CI built timeout caused by the aks-periscope pods crash-looping.

The crashing behaviour was introduced by #96 but only occurs intermittently because the the pods are briefly in Ready state, so the kubectl -n aks-periscope wait po --all --for condition=ready --timeout=240s command sometimes completes successfully.

Previously to the above change, the createContainerURL function used to fail with an error due to no kubeconfig file existing, meaning that the process would complete (and sleep) without attempting to write anything to blob storage. Following that change, the absence of storage configuration would fail without returning an error, and the process would attempt to write to an empty blob URL, resulting in a panic and crash.

This fixes that function so it explicitly returns an error if storage settings are not configured, which seems like it should be the desired behaviour anyway.

@peterbom peterbom requested a review from Tatsinnit March 6, 2022 23:47
@codecov-commenter
Copy link

Codecov Report

Merging #156 (a895f15) into master (48270ee) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #156   +/-   ##
=======================================
  Coverage   68.98%   68.98%           
=======================================
  Files          11       11           
  Lines         345      345           
=======================================
  Hits          238      238           
  Misses         85       85           
  Partials       22       22           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48270ee...a895f15. Read the comment docs.

Copy link
Member

@Tatsinnit Tatsinnit left a comment

Choose a reason for hiding this comment

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

☕️🎉

@Tatsinnit Tatsinnit merged commit 855c9c9 into Azure:master Mar 7, 2022
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