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

Update boskos stateful set to use RollingUpdate (default @v1) #13594

Merged
merged 1 commit into from
Sep 6, 2019
Merged

Update boskos stateful set to use RollingUpdate (default @v1) #13594

merged 1 commit into from
Sep 6, 2019
+5 −37

Conversation

sebastienvas
Copy link
Contributor

@sebastienvas sebastienvas commented Jul 25, 2019 •
edited
Loading

Remove all the persistent volumes things, since we have using CRDs for more than a year now.

Fixes ##11956

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 25, 2019
@k8s-ci-robot k8s-ci-robot requested review from ixdy and krzyzacy July 25, 2019 00:03
@k8s-ci-robot k8s-ci-robot added area/boskos Issues or PRs related to code in /boskos sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Jul 25, 2019
@sebastienvas
Copy link
Contributor Author

/assign @Katharine

@krzyzacy
Copy link
Member

/lgtm
/hold

@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 Jul 26, 2019
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 8320f0242159894b11361ebbdba0d02a25bd5932

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 26, 2019
@krzyzacy
Copy link
Member

krzyzacy commented Aug 1, 2019

(feel free to hold cancel when you are around)

@sebastienvas
Copy link
Contributor Author

is this going to auto deploy ?
I don't think I have access to that cluster

@Katharine
Copy link
Member

I don't believe this will auto-deploy. Oncall (currently @mirandachrist) should probably deploy this after it merges.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: clarketm, sebastienvas

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

@stevekuznetsov
Copy link
Contributor

Why isn't boskos a Deployment?

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 5, 2019
@sebastienvas
Copy link
Contributor Author

Good point. However in order to deploy this we want to make sure the stateful set is deleted before. Not sure how on call wants to handle this.

@stevekuznetsov
Copy link
Contributor

I think it would be best to list the kubectl commands you expect here so that on-call can just execute them and post the output. I think we'd want to manually delete the StatefulSet, then the PV, then apply the Deployment.

@sebastienvas
Copy link
Contributor Author

sebastienvas commented Sep 6, 2019 •
edited
Loading

correct:
kubectl delete -n test-pods statefulset boskos
wait for boskos-0 pod to be gone in test-pods ns
kubectl delete -n test-pods pv boskos-storage
kubectl apply -f boskos/deployment.yaml

@chases2
Copy link
Contributor

chases2 commented Sep 6, 2019

/hold cancel
/lgtm

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

LGTM label has been added.

Git tree hash: 81241b6b27cebb3c40a214c02f7719749d3948e4

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 6, 2019
@k8s-ci-robot k8s-ci-robot merged commit fc85258 into kubernetes:master Sep 6, 2019
@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Sep 6, 2019
@chases2
Copy link
Contributor

chases2 commented Sep 9, 2019

In order to remove the persistent volume, the persistent volume claim also needed to be removed. On the k8s build cluster, this was named boskos-volume-boskos-0.

statefulset.apps "boskos" deleted

persistentvolume "boskos-storage" deleted

clusterrole.rbac.authorization.k8s.io/boskos unchanged
serviceaccount/boskos unchanged
clusterrolebinding.rbac.authorization.k8s.io/crd-creator unchanged
deployment.apps/boskos created

kubectl delete -n test-pods pvc boskos-volume-boskos-0
persistentvolumeclaim "boskos-volume-boskos-0" deleted

@sebastienvas
Copy link
Contributor Author

Thanks @chases2

@chases2 chases2 mentioned this pull request Sep 9, 2019
Closed
@chaodaiG chaodaiG mentioned this pull request Dec 11, 2019
Closed
chaodaiG added a commit to chaodaiG/knative-test-infra that referenced this pull request Dec 11, 2019
@chaodaiG
Switching Boskos to deployment, as well as removing persistent volume
08282b1 
This change is to make our Boskos deployment consistent with upstream, almost a mirror of kubernetes/test-infra#13594
@chaodaiG chaodaiG mentioned this pull request Dec 11, 2019
Merged
knative-prow-robot pushed a commit to knative/test-infra that referenced this pull request Dec 12, 2019
@chaodaiG @knative-prow-robot
Switching Boskos to deployment, as well as removing persistent volume (…
a00f8b8 
…#1584)

This change is to make our Boskos deployment consistent with upstream, almost a mirror of kubernetes/test-infra#13594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clarketm clarketm clarketm approved these changes

@ixdy ixdy Awaiting requested review from ixdy

@krzyzacy krzyzacy Awaiting requested review from krzyzacy

Assignees

@Katharine Katharine

@krzyzacy krzyzacy

@chases2 chases2

@clarketm clarketm

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/boskos Issues or PRs related to code in /boskos 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/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.16
Development

Successfully merging this pull request may close these issues.
7 participants
@sebastienvas @krzyzacy @k8s-ci-robot @Katharine @stevekuznetsov @chases2 @clarketm