-
Notifications
You must be signed in to change notification settings - Fork 212
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
RFC-0030 - Add support to Diego for file based service bindings #942
base: develop
Are you sure you want to change the base?
Changes from 17 commits
07ac037
9b4c77a
6cb0e5b
5e45a22
99dd7b2
c96537a
e107132
46162b6
c530058
2b925af
56f187b
beb5a62
f0b8a59
b365cbc
1709145
0696ffe
f97689b
95aa7c4
1af6dda
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
#!/usr/bin/env bash | ||
|
||
<% | ||
_max_containers = 250 | ||
if_p("diego.rep.max_containers") do |value| | ||
_max_containers = value | ||
end | ||
if _max_containers <= 0 | ||
raise "The max_containers prop should be a positive integer" | ||
end | ||
%> | ||
max_containers=<%= _max_containers %> | ||
|
||
# Define the service binding root directory | ||
volume_mounted_files="/var/vcap/data/rep/shared/garden/volume_mounted_files" | ||
|
||
# Calculate the size for the tmpfs (1MB per container) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is going to increase the size of the diego footprint on the cell, which will reduce space for the containers themselves. ❓ Is additional space taken into account when the rep reports the resources on the cell to the BBS? It should be taken into account when this property is set to "auto". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello, that's new for me. Would you elaborate further on this? Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my understanding, this https://github.com/cloudfoundry/executor/blob/main/initializer/configuration/configuration.go#L115 should be extended to add number_of_containers * 1MB to account for the 'overhead' that 'volume_mounted_files' will bring. Am I correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! That looks correct to me. |
||
root_tmpfs_size=$((max_containers * 1))M | ||
|
||
# Ensure the root directory is safely removed and recreated | ||
if [ -d "$volume_mounted_files" ]; then | ||
# Ensure the root directory is unmounted before removing it | ||
if mountpoint -q "$volume_mounted_files"; then | ||
fuser -k "$volume_mounted_files" | ||
umount -l "$volume_mounted_files" | ||
fi | ||
|
||
sleep 10 | ||
|
||
rm -rf "$volume_mounted_files" | ||
fi | ||
|
||
mkdir -p "$volume_mounted_files" | ||
|
||
# Mount the root tmpfs | ||
mount -t tmpfs -o size="$root_tmpfs_size" tmpfs "$volume_mounted_files" || exit 1 | ||
|
||
# Set permissions and ownership for the root directory and all subdirectories | ||
chmod 0700 "$volume_mounted_files" | ||
chown vcap:vcap "$volume_mounted_files" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks to me like an unwanted change here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. I need to run the Diego Release test suite first. After it passes, the modules will be reverted.