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

Add cloudbuild.yaml for staging #2125

Merged
merged 1 commit into from Feb 24, 2023

Conversation

mnaser
Copy link
Contributor

@mnaser mnaser commented Feb 21, 2023

What this PR does / why we need it:
Adds a cloudbuild.yaml so we can start pushing things out of DockerHub

Which issue this PR fixes(if applicable):
related to #1540

Special notes for reviewers:

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 21, 2023
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 21, 2023
@k8s-ci-robot
Copy link
Contributor

Hi @mnaser. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 21, 2023
@jichenjc
Copy link
Contributor

could you please help elaborate the way to solve the issue you pointed out with this PR? Thanks

@mnaser

@jichenjc
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 21, 2023
@zetaab
Copy link
Member

zetaab commented Feb 22, 2023

@mnaser could you please open little bit how this is supposed to work? As we are using github actions in this repo and so on, we do not have any cloudbuild access.

Copy link
Contributor

@mdbooth mdbooth left a comment

Choose a reason for hiding this comment

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

Here's what we currently do for release builds:

REGISTRY=docker.io/k8scloudprovider ARCHS='amd64 arm arm64 ppc64le s390x' GOOS=linux VERSION=${{ steps.get_version.outputs.VERSION }} make upload-images

This looks correct to me and I recall this being a required part of the release puzzle. I can't find the docs, though. A link in the PR would be nice.

/lgtm

- name: gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20220609-2e4c91eb7e
entrypoint: make
env:
- REGISTRY=gcr.io/k8s-staging-cloud-provider-os
Copy link
Contributor

Choose a reason for hiding this comment

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

_GIT_TAG: '12345'
# _PULL_BASE_REF will contain the ref that was pushed to to trigger this
# build - a branch like 'master' or 'release-0.2', or a tag like 'v0.2'.
_PULL_BASE_REF: 'master'
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not using this. However, as it seems to be a standard substitution probably simpler to leave it in to reduce friction if we want to use it in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

seems correct? CPO didn't switch to main currently, still using master as primary dev branch..

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 22, 2023
@zetaab
Copy link
Member

zetaab commented Feb 22, 2023

I can easily add our images to k8s.io registry. However, I do not have credentials or contact where to ask those. So if someone could help with that.

@mdbooth
Copy link
Contributor

mdbooth commented Feb 22, 2023

I can easily add our images to k8s.io registry. However, I do not have credentials or contact where to ask those. So if someone could help with that.

I think Prow already has them? IIUC they are managed by the k8s org.

@mdbooth
Copy link
Contributor

mdbooth commented Feb 22, 2023

@zetaab @mnaser Found the reference: https://github.com/kubernetes/test-infra/blob/master/config/jobs/image-pushing/README.md

E.g. Here's how it works in CAPO, which is in kubernetes-sigs but it works the same way:

There's a postsubmit job which runs after commits to main or release branches: https://github.com/kubernetes/test-infra/blob/c7adcaabf4800775a2b3ccfdff5545c4119a5c43/config/jobs/image-pushing/k8s-staging-cluster-api.yaml#L184-L208

It automatically pushes a new image to the staging repo by running the cloud build. Maintainers then promote staging images to production manually by pushing a PR to update https://github.com/kubernetes/k8s.io/blob/main/k8s.gcr.io/images/k8s-staging-capi-openstack/images.yaml

It's documented in our release doc: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/main/RELEASE.md

@mdbooth
Copy link
Contributor

mdbooth commented Feb 22, 2023

I can easily add our images to k8s.io registry. However, I do not have credentials or contact where to ask those. So if someone could help with that.

Incidentally, I believe the answer to this is suggested in the image-pushing job:

https://github.com/kubernetes/test-infra/blob/e8be84d7787b4864ac5b53130defa743625eecc9/config/jobs/image-pushing/k8s-staging-cloud-provider-openstack.yaml#L5

      cluster: k8s-infra-prow-build-trusted

My guess is that the reason you aren't permitted to run arbitrary code in the trusted cluster is that it has these credentials. Guessing this also implies they won't share them except via prow.

@jichenjc
Copy link
Contributor

@mdbooth thanks for the detailed info

To confirm my understanding, So we need get this PR merged then we have a pre-requirement satisfied
then get https://github.com/kubernetes/test-infra/pull/28817/files merged
after that we will have a similar output / method of release corresponding to CAPO ?

@mnaser
Copy link
Contributor Author

mnaser commented Feb 23, 2023

Based on my understanding is that yes, this is the first step to being able to upload into the staging repo.

@mdbooth
Copy link
Contributor

mdbooth commented Feb 23, 2023

@mnaser Probably want to remove the word 'fixes' before #1540 because IIUC it'll close the issue again. I think this is just step 1.

@jichenjc
Copy link
Contributor

I updated the commit message and let's get this

@jichenjc
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jichenjc

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2023
@k8s-ci-robot k8s-ci-robot merged commit e5a914a into kubernetes:master Feb 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants