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.

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

[REP] Make max_containers prop configurable #876

Merged
merged 1 commit into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jobs/rep/spec
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ properties:
description: "Timeout in seconds to receive a response to the keepalive ping. If a response is not received within this time, the locket client will reconnect to another server."
default: 22

diego.rep.max_containers:
description: "Maximum container capacity per rep"
default: 250

enable_declarative_healthcheck:
description: "When set, enables the rep to prefer the LRP CheckDefinition to healthcheck instances over the Monitor action. Requires Garden-Runc v1.10.0+"
default: false
Expand Down
12 changes: 11 additions & 1 deletion jobs/rep/templates/setup_mounted_data_dirs.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,17 @@ rm -rf "${old_shared_data_dir}"
# add another 4096 to account for the temp files used to do atomic replacement #141163257
instance_cert_and_key_size=10240
instance_ca_size=$(wc -c ${conf_dir}/certs/rep/instance_identity.crt | cut -d' ' -f1)
max_containers=250
<%
_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 %>

instance_tmpfs_size=$((($instance_ca_size + $instance_cert_and_key_size) * $max_containers))

instance_identity_dir=${garden_shared_dir}/instance_identity
Expand Down
119 changes: 67 additions & 52 deletions spec/rep_template_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,66 +10,68 @@
let(:release) { Bosh::Template::Test::ReleaseDir.new(release_path) }
let(:job) { release.job('rep') }

describe 'rep.json.erb' do
let(:deployment_manifest_fragment) do
{
'bpm' => {
'enabled' => 'true'
},
'diego' => {
'executor' => {
'instance_identity_ca_cert' => 'CA CERT',
'instance_identity_key' => 'CA KEY'
},
'rep' => {
'locket' => {
'client_keepalive_time' => 10,
'client_keepalive_timeout' => 22
},
'preloaded_rootfses' => %w[
cflinuxfs3
cflinuxfs4
]
}
let(:deployment_manifest_fragment) do
{
'bpm' => {
'enabled' => 'true'
},
'diego' => {
'executor' => {
'instance_identity_ca_cert' => 'CA CERT',
'instance_identity_key' => 'CA KEY'
},
'containers' => {
'proxy' => {
'enabled' => 'true',
'require_and_verify_client_certificates' => 'true',
'trusted_ca_certificates' => [
'GOROUTER CA',
'SSH PROXY CA'
],
'verify_subject_alt_name' => [
'gorouter.service.cf.internal',
'ssh-proxy.service.cf.internal'
]
'rep' => {
'max_containers' => 250,
'locket' => {
'client_keepalive_time' => 10,
'client_keepalive_timeout' => 22
},
'trusted_ca_certificate' => [
'DIEGO INSTANCE CA',
'CREDHUB CA',
'UAA CA'
'preloaded_rootfses' => %w[
cflinuxfs3
cflinuxfs4
]
}
},
'containers' => {
'proxy' => {
'enabled' => 'true',
'require_and_verify_client_certificates' => 'true',
'trusted_ca_certificates' => [
'GOROUTER CA',
'SSH PROXY CA'
],
'verify_subject_alt_name' => [
'gorouter.service.cf.internal',
'ssh-proxy.service.cf.internal'
]
},
'enable_consul_service_registration' => 'false',
'enable_declarative_healthcheck' => 'true',
'loggregator' => 'LOGREGATOR PROPS',
'tls' => {
'ca_cert' => 'CA CERT',
'cert' => 'CERT',
'key' => 'KEY'
},
'logging' => {
'format' => {
'timestamp' => 'rfc3339'
}
'trusted_ca_certificate' => [
'DIEGO INSTANCE CA',
'CREDHUB CA',
'UAA CA'
]
},
'enable_consul_service_registration' => 'false',
'enable_declarative_healthcheck' => 'true',
'loggregator' => 'LOGREGATOR PROPS',
'tls' => {
'ca_cert' => 'CA CERT',
'cert' => 'CERT',
'key' => 'KEY'
},
'logging' => {
'format' => {
'timestamp' => 'rfc3339'
}
}
end
}
end

let(:rendered_template) { template.render(deployment_manifest_fragment) }

describe 'rep.json.erb' do
let(:template) { job.template('config/rep.json') }
let(:rendered_template) { template.render(deployment_manifest_fragment) }


context 'check if locket keepalive time is bigger than the timeout' do
it 'fails if the keepalive time is bigger than timeout' do
deployment_manifest_fragment['diego']['rep']['locket']['client_keepalive_time'] = 23
Expand All @@ -79,4 +81,17 @@
end
end
end

describe 'setup_mounted_data_dirs.erb' do
let(:template) { job.template('bin/setup_mounted_data_dirs') }

context 'checks the max_containers value' do
it 'raises an error if max_containers is <= 0' do
deployment_manifest_fragment['diego']['rep']['max_containers'] = -10
expect do
rendered_template
end.to raise_error(/The max_containers prop should be a positive integer/)
end
end
end
end