-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix(gcs): backoff bool should be false if error is transient #6577
Conversation
Signed-off-by: AntoineDao <antoinedao1@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #6577 +/- ##
==========================================
- Coverage 48.62% 48.57% -0.06%
==========================================
Files 261 261
Lines 18945 18961 +16
==========================================
- Hits 9212 9210 -2
- Misses 8714 8732 +18
Partials 1019 1019
Continue to review full report at Codecov.
|
I'm not sure what the linting error is here... Any help figuring what went wrong? I tested these changes on our staging cluster and it works as expected. The GCS bucket is publicly available so you should be able to test them as is on your own cluster or on minkube if you want to test the changes to ensure it works as expected. I'm not sure how to test the retry behavior however but I could observe it retrying for optional artifacts before this fix so I am confident it will still work as expected. Optional GCS Artifact does not throw error on not foundapiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
namespace: argo
generateName: gcs-test-artifact-exists-
spec:
entrypoint: gcs-test
templates:
- name: gcs-test
inputs:
artifacts:
- name: my-art
path: /my-artifact
optional: true
gcs:
bucket: pollination-public
key: blobs/argo-test/not-exists.txt
container:
image: ubuntu:latest
command: [sh, -c]
args: ["cat /my-artifact"] Non-Optional GCS Artifact throws error on not foundapiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
namespace: argo
generateName: gcs-test-artifact-exists-
spec:
entrypoint: gcs-test
templates:
- name: gcs-test
inputs:
artifacts:
- name: my-art
path: /my-artifact
gcs:
bucket: pollination-public
key: blobs/argo-test/not-exists.txt
container:
image: ubuntu:latest
command: [sh, -c]
args: ["cat /my-artifact"] Existing GCS artifact is downloaded correctlyapiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
namespace: argo
generateName: gcs-test-artifact-exists-
spec:
entrypoint: gcs-test
templates:
- name: gcs-test
inputs:
artifacts:
- name: my-art
path: /my-artifact
gcs:
bucket: pollination-public
key: blobs/argo-test/exists.txt
container:
image: ubuntu:latest
command: [sh, -c]
args: ["cat /my-artifact"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. You’ll need to fix the formatting to pass the checks.
Signed-off-by: AntoineDao <antoinedao1@gmail.com>
Signed-off-by: AntoineDao <antoinedao1@gmail.com>
Fixes #6576
I'm not really sure how to write proper
e2e
orunit
tests for this fix as it involves stubbing the GCS API... I am deploying this fix to our staging cluster and testing it just now. I'll comment here with the tests carried out and the outcomes.