Skip to content

add script to create conformance buckets, add bucket for capo#459

Merged
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
sbueringer:pr-add-conformance-bucket-script
Feb 4, 2020
Merged

add script to create conformance buckets, add bucket for capo#459
k8s-ci-robot merged 5 commits intokubernetes:masterfrom
sbueringer:pr-add-conformance-bucket-script

Conversation

@sbueringer
Copy link
Member

For context see: kubernetes/test-infra#15081

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 10, 2019
@dims
Copy link
Member

dims commented Nov 10, 2019

/assign @listx @thockin

cc @BenTheElder

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

I think this is a bit under-specified - it seems to me that conformance is closer to "prod" than staging in terms of retention/deletion policy?

@BenTheElder
Copy link
Member

thanks for the PR!!

@thockin
Copy link
Member

thockin commented Nov 11, 2019 via email

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Question for the group:

Staging requires a GCP project per staging, because it uses GCR and that's implictly 1:1 with project.

This does not use GCR - it coud technically be all in one project, different buckets.

Other than a smaller list of projects, it isn't significantly better or worse. My inclination is to leave this as-is, but I thought I would bring it up so we don't second-guess later.

@thockin
Copy link
Member

thockin commented Nov 11, 2019

I'm approving, but @BenTheElder gets final LGTM

/approve

@thockin thockin added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2019
@justaugustus
Copy link
Member

@thockin --

This does not use GCR - it coud technically be all in one project, different buckets.

I was coming here to say this!
Seems that we're sprawling a bit here, and I think we'd be just as easily served with a single conformance GCP project with separate buckets or conformance buckets within existing projects e.g., for CAPI, why not store conformance info within their primary GCP project or the respective provider projects?

@sbueringer
Copy link
Member Author

@justaugustus

At least in case of ClusterAPI OpenStack the only GCP project which already exists (afaik) is named k8s-staging-capi-openstack which would probably be not the correct on the add a bucket for conformance test results.

Apart from that I don't really know how Kubernetes GCP projects/buckets are usually organized. So I let you discuss and adjust the script accordingly :)

@BenTheElder
Copy link
Member

BenTheElder commented Nov 11, 2019

Currently I maintain a single GCP project with multiple buckets [for conformance test results] and service accounts 1:1, with specific IAM permissions to the buckets matched to service accounts.

I'm not sure which pattern we want going forward.

@thockin
Copy link
Member

thockin commented Nov 15, 2019

I'd be OK with a single k8s-conformance or k8s-conformance-results project with N buckets and a mailing list IAM to write to each bucket

@sbueringer
Copy link
Member Author

I'd be OK with a single k8s-conformance or k8s-conformance-results project with N buckets and a mailing list IAM to write to each bucket

Okay so one project with multi buckets. Just that I get it correctly. Only one google group for the project or one per bucket?

@BenTheElder
Copy link
Member

let's do one project with a group per bucket, someone will need to own the project as well though.

@sbueringer
Copy link
Member Author

sbueringer commented Nov 15, 2019

@BenTheElder Is there's already an existing group I can reuse or should I create a new one?

@BenTheElder
Copy link
Member

BenTheElder commented Nov 15, 2019 via email

@sbueringer
Copy link
Member Author

@thockin @BenTheElder @dims

I updated the script to use a single project for all buckets. For now I used the "k8s-infra-conform@kubernetes.io" group for the project. If we want to use this group, instead of reusing an existing one I also have to define this group in the groups.yaml (which I haven't done yet).

I'm not sure which rights are necessary on the project. I used empower_group_as_viewer again like in the previous version of the script.

@sbueringer
Copy link
Member Author

@thockin @BenTheElder @dims

I updated the script to use a single project for all buckets. For now I used the "k8s-infra-conform@kubernetes.io" group for the project. If we want to use this group, instead of reusing an existing one I also have to define this group in the groups.yaml (which I haven't done yet).

I'm not sure which rights are necessary on the project. I used empower_group_as_viewer again like in the previous version of the script.

Just wanted to let you know that I'm on vacation until end of Dezember. I would continue this PR then. If anyone wants to takeover in the meantime, be my guest :)

@dims
Copy link
Member

dims commented Nov 23, 2019

/assign @dims

@dims
Copy link
Member

dims commented Nov 25, 2019

@BenTheElder @sbueringer let's use a new one

@BenTheElder
Copy link
Member

SGTM

@dims
Copy link
Member

dims commented Dec 6, 2019

/hold

let's wait for @sbueringer to get back :)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2019
Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

Otherwise this looks ok to me


PROJECT="k8s-conform"

PROJECT_VIEWER="k8s-infra-conform@kubernetes.io"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this, do we? What is it for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be a leftover from copying this script. I removed PROJECT_VIEWER

@sbueringer
Copy link
Member Author

@thockin please take another look :)

@sbueringer
Copy link
Member Author

@thockin please take another look :)

push :)
@dims @BenTheElder

@thockin
Copy link
Member

thockin commented Feb 4, 2020

Thanks!

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 4, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@thockin thockin removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2020
@k8s-ci-robot k8s-ci-robot merged commit 78dfa95 into kubernetes:master Feb 4, 2020
@thockin
Copy link
Member

thockin commented Feb 5, 2020

I am actuating this. One function got renamed, fix coming.

@sbueringer sbueringer deleted the pr-add-conformance-bucket-script branch February 15, 2020 12:51
@dims dims mentioned this pull request Mar 2, 2021
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants