search

cloudfoundry-incubator / kubo-deployment

Contains manifests used to deploy Cloud Foundry Container Runtime
https://www.cloudfoundry.org/container-runtime/
Apache License 2.0
275 stars 114 forks source link

Set the root-dir to a partition that is scalable #384

Closed pgoodwin closed 5 years ago

pgoodwin commented 5 years ago

...so that it is possible to avoid eviction thresholds when log files grow

What this does / why we need it: Fixes the bug described here: https://www.pivotaltracker.com/story/show/164157527

How can this be verified? Deploy CFCR. Create and run a pod that writes data to an emptyDir type volume (sample code can be found here, and observe that it writes data into the /var/vcap/data/kubelet mount point. The specific path on each worker VM will be something like: /var/vcap/data/kubelet/pods/xxxxxxx-xxxx-xxxxxxxx/volumes/kubernetes.io~empty-dir/simple-vol/file.txt

Is there any change in kubo-release? No

Is there any change in kubo-ci? No (but should there be?)

Does this affect upgrade, or is there any migration required? No

Which issue(s) does this fixes? Tracker #164157527

Release note: TBD by PMs

cfdreddbot commented 5 years ago

:x: Hey pgoodwin!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA). Click here for details on the CLA process.

The following github user @betarelease is not covered by a CLA.

After the CLA process is complete, this pull request will need to be closed & reopened. DreddBot will then validate the CLA(s).

pgoodwin commented 5 years ago

/review

pgoodwin commented 5 years ago

cc @imikushin

pgoodwin commented 5 years ago

Woops. Wrong repo. Wrong org.

pgoodwin commented 5 years ago

OK, it is the right repo! @tvs PTAL

cfdreddbot commented 5 years ago

:x: Hey pgoodwin!

All pull request submitters and commit authors must have a Contributor License Agreement (CLA). Click here for details on the CLA process.

The following github user @betarelease is not covered by a CLA.

After the CLA process is complete, this pull request will need to be closed & reopened. DreddBot will then validate the CLA(s).

pgoodwin commented 5 years ago

We also changed the base branch to develop as requested in Slack.

pgoodwin commented 5 years ago

Ivan and I are working on an integration test right now. I agree that the addition of the context block on a test is of value. I think the integration test will get us everything we want here.

On Fri, Mar 15, 2019, 2:50 PM Travis Hall notifications@github.com wrote:

@tvs commented on this pull request.

In manifests/cfcr.yml https://github.com/cloudfoundry-incubator/kubo-deployment/pull/384#discussion_r266157340 :

@@ -352,6 +352,7 @@ instance_groups: docker-endpoint: unix:///var/vcap/sys/run/docker/docker.sock kubeconfig: /var/vcap/jobs/kubelet/config/kubeconfig network-plugin: cni

  • root-dir: /var/vcap/data/kubelet

My intent was that the tests would serve as a form of documentation for why that flag is present. Of course one could just modify the thing in two locations, but hopefully the context of the test would at least drive the modification to remain consistent with our purpose.

An integration test to do the same would be quite cumbersome, though if structured well, could be more resilient to arbitrary changes in BOSH's mount scheme. Maybe we should consider adding a test in kubo-ci instead.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/cloudfoundry-incubator/kubo-deployment/pull/384#discussion_r266157340, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGVJ4Xr4NjRuFi625xdZbMO2clkx6kHks5vXBWegaJpZM4b04pL .