-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Support publishing CI artifacts to S3 #16659
base: master
Are you sure you want to change the base?
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
8e2ecb8
to
85e8356
Compare
@@ -34,6 +36,7 @@ import ( | |||
const ( | |||
defaultJobName = "pull-kops-e2e-kubernetes-aws" | |||
defaultGCSPath = "gs://k8s-staging-kops/pulls/%v/pull-%v" | |||
defaultS3Path = "s3://k8s-infra-kops-ci-results/pulls/%v/pull-%v" |
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.
Created by kubernetes/k8s.io#6950
85e8356
to
35c1d03
Compare
@rifelpet: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
We want kubetest2-kops to dynamically create a temporary s3 bucket when the presubmit is running, similar to this code I made the IAM changes, but it allows presubmits to write to our CI bucket which isn't ideal(existing behaviour we should change). |
Yes I see the migrated job on your PR passed, using k8s-staging-kops: I believe this means we could migrate all presubmit jobs to use this, but you're right giving presubmit jobs write permissions to the same bucket used for post-merge artifacts like version markers is not great. We have the |
For now, let's retain the existing behaviour as AWS presubmits have been writing to the k8s-staging-kops bucket and let's try to switch to dynamic buckets on AWS. As for gs://k8s-infra-kops-ci-results, I don't want to use this bucket and we should delete it. |
@@ -271,6 +274,15 @@ gcs-publish-ci: gsutil version-dist-ci | |||
echo "${GCS_URL}/${VERSION}" > ${UPLOAD}/${LATEST_FILE} | |||
gsutil -h "Cache-Control:private, max-age=0, no-transform" cp ${UPLOAD}/${LATEST_FILE} ${GCS_LOCATION} | |||
|
|||
.PHONY: s3-publish-ci | |||
s3-publish-ci: VERSION := ${KOPS_CI_VERSION}+${GITSHA} | |||
s3-publish-ci: version-dist-ci |
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.
We do also have hack/upload
, which might make this easier?
cmd.SetDir(b.KopsRoot) | ||
exec.InheritOutput(cmd) | ||
if err := cmd.Run(); err != nil { | ||
return nil, err | ||
} | ||
|
||
// Get the full path (including subdirectory) that we uploaded to | ||
// It is written by gcs-publish-ci to .build/upload/latest-ci.txt | ||
// It is written by the *-publish-ci make tasks to .build/upload/latest-ci.txt |
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.
It might be nice to spell out the two tasks here (assuming we can't combine them), to make it more explicit and to help with searching...
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.
This actually LGTM. We're probably holding this PR for now (given that the test-infra one is closed), but I think a good reason to upload artifacts to S3 would be to cut down on bandwidth etc costs for testing. But I guess they aren't that large....
We do need to ship this to fix the AWS presubmits publishing to the k8s-staging-kops bucket. We carried over that practice when the jobs were migrated to the community cluster. |
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
Ref: #16637 kubernetes/k8s.io#5127 kubernetes/k8s.io#2625
More context in kubernetes/k8s.io#6950 (comment)