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.

Sign up for GitHub

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

Jump to bottom

prow-build-trusted: Add PVC for publishing-bot #5685

Merged
merged 1 commit into from
Aug 17, 2023
Merged

prow-build-trusted: Add PVC for publishing-bot #5685

merged 1 commit into from
Aug 17, 2023

Conversation

ameukam
Copy link
Member

@ameukam ameukam commented Aug 4, 2023

Related to:

Add a PVC that will be used publishing-bot as a prowjob

@ameukam
prow-build-trusted: Add PVC for publishing-bot
3a568f3 
Related to:
  - kubernetes/publishing-bot#353

Add a PVC that will be used publishing-bot as a prowjob

Signed-off-by: Arnaud Meukam <[email protected]>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 4, 2023
@ameukam
Copy link
Member Author

ameukam commented Aug 4, 2023

cc @akhilerm

@k8s-ci-robot k8s-ci-robot added area/infra Infrastructure management, infrastructure design, code in infra/ area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters labels Aug 4, 2023
@k8s-ci-robot k8s-ci-robot added area/terraform Terraform modules, testing them, writing more of them, code in infra/gcp/clusters/ size/S Denotes a PR that changes 10-29 lines, ignoring generated files. sig/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. labels Aug 4, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ameukam

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 sig/testing Categorizes an issue or PR as relevant to SIG Testing. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 4, 2023
akhilerm
akhilerm reviewed Aug 7, 2023
View reviewed changes
...form/k8s-infra-prow-build-trusted/prow-build-trusted/resources/test-pods/test-pods-pvcs.yaml
Copy link
Member

Choose a reason for hiding this comment

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

q: There can be cases where the contents stored in the publishing bot PV can get corrupted sometime and we will have to delete and recreate it.

Ref: https://github.com/kubernetes/publishing-bot/blob/master/production.md#what-is-the-persistent-volume-for

In such a scenario, how can someone easily delete and recreate the PVC ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the publishing-bot pod is handled by the prowjob, I suspect (I think I'm wrong) that for each run of the prowjob we'll get a new PV (or do we want to create the PV manually? ). If that's the case, cleanup can handled differently. One alternative could be the usage of ephemeral storage in the podSpec.

Copy link
Member

Choose a reason for hiding this comment

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

Nope. We dont to have a new PV for everyrun, that will defeat the purpose. The PV is used so that we dont have to download all the repos everytime. And ephemeral storage also wont suffice our needs. A PV is what we need, and whatever changes in this are good. I was thinking of methods on how to clean it incase if it gets corrupted.

Copy link
Member

Choose a reason for hiding this comment

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

Anyways, the cleanup part we can think of later in a separate PR once the publishing-bot is migrated to test-infra.

IMHO, for adding the PVC this is good. Thank you

...form/k8s-infra-prow-build-trusted/prow-build-trusted/resources/test-pods/test-pods-pvcs.yaml
labels:
app: publisher
spec:
storageClassName: "premium-rwo"
Copy link
Member

Choose a reason for hiding this comment

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

Is this same as ssd storage class?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

akhilerm
akhilerm reviewed Aug 7, 2023
View reviewed changes
...form/k8s-infra-prow-build-trusted/prow-build-trusted/resources/test-pods/test-pods-pvcs.yaml
kind: PersistentVolumeClaim
metadata:
name: publishing-bot-pvc
namespace: test-pods
Copy link
Member

Choose a reason for hiding this comment

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

q: Is the same namespace in which all the prow test jobs run?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

@akhilerm
Copy link
Member

/lgtm

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

q: @ameukam Is this change rolled out manually. or is the PVC created automatically?

@k8s-ci-robot k8s-ci-robot merged commit 22aa226 into kubernetes:main Aug 17, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.29 milestone Aug 17, 2023
@ameukam
Copy link
Member Author

ameukam commented Sep 4, 2023

q: @ameukam Is this change rolled out manually. or is the PVC created automatically?

@akhilerm it was auto-deployed:

$ kubectl get pvc -n test-pods
NAME                 STATUS    VOLUME   CAPACITY   ACCESS MODES   STORAGECLASS   AGE
publishing-bot-pvc   Pending                                      premium-rwo    18d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akhilerm akhilerm akhilerm left review comments

@jbpratt jbpratt Awaiting requested review from jbpratt

@justaugustus justaugustus Awaiting requested review from justaugustus

Assignees

@akhilerm akhilerm

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/infra Infrastructure management, infrastructure design, code in infra/ area/prow Setting up or working with prow in general, prow.k8s.io, prow build clusters area/terraform Terraform modules, testing them, writing more of them, code in infra/gcp/clusters/ 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. sig/k8s-infra Categorizes an issue or PR as relevant to SIG K8s Infra. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.29
Development

Successfully merging this pull request may close these issues.
3 participants
@ameukam @k8s-ci-robot @akhilerm