-
Notifications
You must be signed in to change notification settings - Fork 2.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
ci-k8sio-audit periodic job #20742
ci-k8sio-audit periodic job #20742
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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/test-infra repository. I understand the commands that are listed here. |
I’m trying to use ./config/pj-on-kind.sh to verify a job that needs to use gcloud. It will eventually run in run in the k8s-infra-prow-build-trusted cluster and |
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.
Some suggestions
testgrid-alert-email: [email protected] | ||
testgrid-num-failures-to-alert: '1' |
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.
I would change these while you're iterating on this job
./audit-gcp.sh | ||
git diff | ||
git add --all | ||
git commit -m "audit: update as of $(date +%Y-%m-%d)" |
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.
As a start, you could have this job fail if there are changes to commit, and pass if there are none
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.
I may swing back around to that. Trying to get a full run to ensure all the tooling works.
8de871c
to
d7c5700
Compare
/retitle ci-k8sio-audit periodic job |
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.
Some suggestions
- -c | ||
- | | ||
echo "Some initial debuging to figure out what it looks like here" >&2 | ||
pwd |
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.
periodic jobs will have the working directory set to the root of the repo specified by the first ref in extra_refs (if specified)
ref: https://github.com/kubernetes/test-infra/blob/master/prow/pod-utilities.md
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.
Thanks, that's useful!
config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-trusted.yaml
Outdated
Show resolved
Hide resolved
git config user.name "Kubernetes Prow Robot" | ||
git config user.email "[email protected]" |
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 should correspond to the GH_USER being used, and I would advise we not use k8s.ci.robot
echo -n "Calculate github user from token: " >&2 | ||
GH_TOKEN=$(cat /etc/github-token/oauth) | ||
GH_USER=$(curl -H "Authorization: token $GH_TOKEN" "https://api.github.com/user" 2>/dev/null | sed -n "s/\s\+\"login\": \"\(.*\)\",/\1/p") | ||
FORK_GH_BRANCH=autoaudit-${PROW_INSTANCE_NAME:-prow} |
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.
not blocking but what's the purpose of the PROW_INSTANCE_NAME substitution?
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's a bit of copy-pasta from here:
test-infra/prow/cmd/autobump/autobump.sh
Lines 29 to 31 in ff516b7
# Set this to something more specific if the repo hosts multiple Prow instances. | |
# Must be a valid to use as part of a git branch name. (e.g. no spaces) | |
PROW_INSTANCE_NAME="${PROW_INSTANCE_NAME:-prow}" |
Possibly not necessary as I don't expect there to be multiple FORK_GH_BRANCH, but I suspect this repo does host multiple prow instances.
config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-trusted.yaml
Outdated
Show resolved
Hide resolved
config/jobs/kubernetes/wg-k8s-infra/trusted/wg-k8s-infra-trusted.yaml
Outdated
Show resolved
Hide resolved
volumes: | ||
- name: github | ||
secret: | ||
secretName: oauth-token |
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.
I need to double check if we have this. If we don't, I suggest we not use k8s-ci-robot, and for the short-term use a user under your control (using fejta-bot as the model, which is not an org member)
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.
Hmm... I'm not sure how to load secrets like this into the production prow instance.
I generated an oauth-token
for @cncf-ci and can send that in a secure way as soon as I learn how. :)
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.
secretName: oauth-token | |
secretName: cncf-ci-github-token |
gcloud secrets versions access \
--project=k8s-infra-prow-build-trusted \
--secret=cncf-ci-github-token \
latest | \
kubectl --context=gke_k8s-infra-prow-build-trusted_us-central1_prow-build-trusted \
apply -f -
secret/cncf-ci-github-token created
It's not complete yet, but it may be eaiser to merge and iterate. Co-authored-by: Aaron Crickenberger <[email protected]>
Created a google secret in the format of a k8s secret in the test-pods namespace:
|
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.
Added the secret @hh transferred to me via secret manager, job needs some config updates to match name of/in the secret
volumes: | ||
- name: github | ||
secret: | ||
secretName: oauth-token |
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.
secretName: oauth-token | |
secretName: cncf-ci-github-token |
gcloud secrets versions access \
--project=k8s-infra-prow-build-trusted \
--secret=cncf-ci-github-token \
latest | \
kubectl --context=gke_k8s-infra-prow-build-trusted_us-central1_prow-build-trusted \
apply -f -
secret/cncf-ci-github-token created
echo "Ensure gcloud creds are working" >&2 | ||
gcloud config list | ||
echo -n "Calculate github user from token: " >&2 | ||
GH_TOKEN=$(cat /etc/github-token/oauth) |
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.
GH_TOKEN=$(cat /etc/github-token/oauth) | |
GH_TOKEN=$(cat /etc/github-token/token) |
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.
Fixed in 7663f60
This also adds dynamically pulling the email and name via the token rather than needing to hard code them in the job. Note that it's the default email that is used for the token provided. Also switched to parsing curl output via jq rather than sed. GNU sed and BSD sed act a bit differently and I was getting inconsistent results on OSX vs Linux.
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.
/approve
/lgtm
/wg k8s-infra
/sig testing
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hh, spiffxp 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 |
@hh: Updated the
In response to this:
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/test-infra repository. |
extra_refs: | ||
- org: kubernetes | ||
repo: k8s.io | ||
base_ref: master |
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.
https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/ci-k8sio-audit/1359969296511406080
d'oh, this should have been main
not master
, my bad for missing this on review
Fixes kubernetes/k8s.io#244
Developing using https://github.com/kubernetes/test-infra/blob/master/prow/build_test_update.md#using-pj-on-kindsh