From 7ed8c29f99d5acb51a63b6bdb7c4a5f86c9b4245 Mon Sep 17 00:00:00 2001 From: skovaleff Date: Mon, 22 Oct 2018 18:13:59 -0700 Subject: [PATCH] Add ability to control pam_limits via new module 'limits' 1) 'Values' configures limit settings to be persisted. 2) Previous DivingBell controlled limits those were set but now are gone are cleared. 3) Previous values of newly set limits are backed up to /var/divingbell/limits 4) New limit is applied via adding a separate conf file to /etc/security/limits.d 5) The Doc is updated with appropriate details. 6) Dev env with Vagrant 7) Increase number of expected DaemonSets in 020-test 8) Demo: https://asciinema.org/a/209619 Change-Id: I5efb39c498c2b666b4ba97271b59757f4a0c1ca7 --- Vagrantfile | 29 ++++++ divingbell/templates/bin/_limits.sh.tpl | 109 +++++++++++++++++++++ divingbell/templates/bin/_sysctl.sh.tpl | 2 +- divingbell/templates/configmap-limits.yaml | 30 ++++++ divingbell/templates/daemonset-limits.yaml | 71 ++++++++++++++ divingbell/values.yaml | 27 +++++ doc/source/index.rst | 36 ++++++- tools/gate/scripts/020-test-divingbell.sh | 42 +++++++- 8 files changed, 340 insertions(+), 6 deletions(-) create mode 100644 Vagrantfile create mode 100644 divingbell/templates/bin/_limits.sh.tpl create mode 100644 divingbell/templates/configmap-limits.yaml create mode 100644 divingbell/templates/daemonset-limits.yaml diff --git a/Vagrantfile b/Vagrantfile new file mode 100644 index 0000000..9a840c8 --- /dev/null +++ b/Vagrantfile @@ -0,0 +1,29 @@ +# -*- mode: ruby -*- +# vi: set ft=ruby : + +Vagrant.configure("2") do |config| + config.vm.box = "generic/ubuntu1604" + + [:virtualbox, :parallels, :libvirt, :hyperv].each do |provider| + config.vm.provider provider do |vplh, override| + vplh.cpus = 4 + vplh.memory = 4096 + end + end + + config.vm.synced_folder "./", "/root/deploy/airship-divingbell" + + config.vm.define "dbtest" do |node| + node.vm.hostname = "dbtest" + node.vm.provision :shell, inline: <<-SHELL + #mkdir /root/deploy + #git clone git://git.openstack.org/openstack/airship-divingbell /root/deploy/airship-divingbell + git clone https://git.openstack.org/openstack/openstack-helm-infra /root/deploy/openstack-helm-infra + cd /root/deploy/openstack-helm-infra + ./tools/gate/devel/start.sh full + cd /root/deploy/airship-divingbell/ + ./tools/gate/scripts/010-build-charts.sh + ./tools/gate/scripts/020-test-divingbell.sh + SHELL + end +end diff --git a/divingbell/templates/bin/_limits.sh.tpl b/divingbell/templates/bin/_limits.sh.tpl new file mode 100644 index 0000000..3e8f4e3 --- /dev/null +++ b/divingbell/templates/bin/_limits.sh.tpl @@ -0,0 +1,109 @@ +#!/bin/bash + +{{/* +# Copyright 2018 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +*/}} + +set -e + +cat <<'EOF' > {{ .Values.conf.chroot_mnt_path | quote }}/tmp/limits_host.sh +{{ include "divingbell.shcommon" . }} + +fname_prefix='60-divingbell-' +persist_path='/etc/security/limits.d' + +if [ ! -d "${persist_path}" ]; then + mkdir -p "${persist_path}" +fi + +write_test "${persist_path}" + +add_limits_param(){ + local limit="${1}" + die_if_null "${limit}" ", limit not supplied to function" + local domain="${2}" + die_if_null "${domain}" ", domain not supplied to function" + local type="${3}" + die_if_null "${type}" ", type not supplied to function" + local item="${4}" + die_if_null "${item}" ", item not supplied to function" + local value="${5}" + die_if_null "${value}" ", value not supplied to function" + + file_content="${domain} ${type} ${item} ${value}" + file_name="${fname_prefix}${limit}.conf" + file_path="${persist_path}/${file_name}" + + # Persist the new setting + if [ -f "${file_path}" ] && + [ "$(cat ${file_path})" != "${file_content}" ] || + [ ! -f "${file_path}" ] + then + echo "${file_content}" > "${file_path}" + log.INFO "Limits setting applied: ${file_content}" + else + log.INFO "No changes made to limits param: ${limit}" + fi + + curr_settings="${curr_settings}${file_name}"$'\n' +} + +{{- range $index, $limit := .Values.conf.limits }} +add_limits_param {{ $index | squote }} {{ $limit.domain | squote }} {{ $limit.type | squote }}\ + {{ $limit.item | squote }} {{ $limit.value | squote }} +{{- end }} + +# Revert any previously applied limits settings which are now absent +prev_files="$(find "${persist_path}" -type f)" +if [ -n "${prev_files}" ]; then + basename -a ${prev_files} | sort > /tmp/prev_settings + echo "${curr_settings}" | sort > /tmp/curr_settings + revert_list="$(comm -23 /tmp/prev_settings /tmp/curr_settings)" + IFS=$'\n' + for orig_limits_setting in ${revert_list}; do + rm "${persist_path}/${orig_limits_setting}" + log.INFO "Reverted limits setting: ${persist_path}/${orig_limits_setting}" + done +fi + +# Print limit settings +# su is a simple and fast way to see applied changes +# bash, bash -c, sudo, setsid didn't work out for me. +su -c "prlimit --noheadings --output RESOURCE,SOFT,HARD" +# The setting is persisted for a new process. +# It's deliberate design decision to let current process be intact. +# For this test it's just test bash process. +# For production case it's limits_host.sh run by DivingBell pod which is in sleep mode. + +if [ -n "${curr_settings}" ]; then + log.INFO 'All limits configuration successfully validated on this node.' +else + log.WARN 'No limits overrides defined for this node.' +fi + +exit 0 +EOF + +chmod 755 {{ .Values.conf.chroot_mnt_path | quote }}/tmp/limits_host.sh +chroot {{ .Values.conf.chroot_mnt_path | quote }} /tmp/limits_host.sh + +sleep 1 +echo 'INFO Putting the daemon to sleep.' + +while [ 1 ]; do + sleep 300 +done + +exit 0 diff --git a/divingbell/templates/bin/_sysctl.sh.tpl b/divingbell/templates/bin/_sysctl.sh.tpl index 4c60c30..5a6d0d1 100644 --- a/divingbell/templates/bin/_sysctl.sh.tpl +++ b/divingbell/templates/bin/_sysctl.sh.tpl @@ -120,7 +120,7 @@ fi if [ -n "${curr_settings}" ]; then log.INFO 'All sysctl configuration successfully validated on this node.' else - log.WARN 'No syctl overrides defined for this node.' + log.WARN 'No sysctl overrides defined for this node.' fi exit 0 diff --git a/divingbell/templates/configmap-limits.yaml b/divingbell/templates/configmap-limits.yaml new file mode 100644 index 0000000..d79c59f --- /dev/null +++ b/divingbell/templates/configmap-limits.yaml @@ -0,0 +1,30 @@ +{{/* +Copyright 2018 The Openstack-Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/}} + +{{- define "divingbell.configmap.limits" }} +{{- $configMapName := index . 0 }} +{{- $envAll := index . 1 }} +{{- with $envAll }} +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ $configMapName }} +data: + limits: |+ +{{ tuple "bin/_limits.sh.tpl" . | include "helm-toolkit.utils.template" | indent 4 }} +{{- end }} +{{- end }} diff --git a/divingbell/templates/daemonset-limits.yaml b/divingbell/templates/daemonset-limits.yaml new file mode 100644 index 0000000..fd239d3 --- /dev/null +++ b/divingbell/templates/daemonset-limits.yaml @@ -0,0 +1,71 @@ +{{/* +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +*/}} + +{{- define "divingbell.daemonset.limits" }} + {{- $daemonset := index . 0 }} + {{- $configMapName := index . 1 }} + {{- $envAll := index . 2 }} + {{- with $envAll }} +--- +apiVersion: extensions/v1beta1 +kind: DaemonSet +metadata: + name: {{ $daemonset }} + annotations: + {{ tuple $envAll | include "helm-toolkit.snippets.release_uuid" }} +spec: +{{ tuple $envAll $daemonset | include "helm-toolkit.snippets.kubernetes_upgrades_daemonset" | indent 2 }} + template: + metadata: + labels: +{{ list $envAll .Chart.Name $daemonset | include "helm-toolkit.snippets.kubernetes_metadata_labels" | indent 8 }} + spec: + hostNetwork: true + hostPID: true + hostIPC: true + containers: + - name: {{ $daemonset }} + image: {{ .Values.images.divingbell }} + imagePullPolicy: {{ .Values.images.pull_policy }} +{{ tuple $envAll $envAll.Values.pod.resources.limits | include "helm-toolkit.snippets.kubernetes_resources" | indent 8 }} + command: + - /tmp/{{ $daemonset }}.sh + volumeMounts: + - name: rootfs-{{ $daemonset }} + mountPath: {{ .Values.conf.chroot_mnt_path }} + - name: {{ $configMapName }} + mountPath: /tmp/{{ $daemonset }}.sh + subPath: {{ $daemonset }} + readOnly: true + securityContext: + privileged: true + volumes: + - name: rootfs-{{ $daemonset }} + hostPath: + path: / + - name: {{ $configMapName }} + configMap: + name: {{ $configMapName }} + defaultMode: 0555 + {{- end }} +{{- end }} +{{- if .Values.manifests.daemonset_limits }} +{{- $daemonset := "limits" }} +{{- $configMapName := "divingbell-limits" }} +{{- $daemonset_yaml := list $daemonset $configMapName . | include "divingbell.daemonset.limits" | toString | fromYaml }} +{{- $configmap_include := "divingbell.configmap.limits" }} +{{- list $daemonset $daemonset_yaml $configmap_include $configMapName . | include "helm-toolkit.utils.daemonset_overrides" }} +{{- end }} diff --git a/divingbell/values.yaml b/divingbell/values.yaml index f29e8e4..8436524 100644 --- a/divingbell/values.yaml +++ b/divingbell/values.yaml @@ -25,6 +25,21 @@ conf: chroot_mnt_path: '/mnt' log_colors: False +## data.values.conf.sysctl +# sysctl: +# fs.suid_dumpable: '0' +## data.values.conf.limits +# limits: +# nofile: +# domain: 'root' +# type: 'soft' +# item: 'nofile' +# value: '101' +# core_dump: +# domain: '0:' +# type: 'hard' +# item: 'core' +# value: 0 pod: lifecycle: upgrades: @@ -46,6 +61,10 @@ pod: enabled: true min_ready_seconds: 0 max_unavailable: 100% + limits: + enabled: true + min_ready_seconds: 0 + max_unavailable: 100% resources: enabled: false ethtool: @@ -76,9 +95,17 @@ pod: requests: memory: "128Mi" cpu: "100m" + limits: + limits: + memory: "128Mi" + cpu: "100m" + requests: + memory: "128Mi" + cpu: "100m" manifests: daemonset_ethtool: true daemonset_mounts: true daemonset_uamlite: true daemonset_sysctl: true + daemonset_limits: true diff --git a/doc/source/index.rst b/doc/source/index.rst index 65d0b80..df01788 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -49,7 +49,8 @@ In order to keep configuration as isolated as possible from other systems that manage common files like /etc/fstab and /etc/sysctl.conf, Divingbell daemonsets manage all of their configuration in separate files (e.g. by writing unique files to /etc/sysctl.d or defining unique Systemd units) to avoid potential -conflicts. +conflicts. Another example is limit management, Divingbell daemonset writes +separate files to /etc/security/limits.d. To maximize robustness and utility, the daemonsets in this chart are made to be idempotent. In addition, they are designed to implicitly restore the original @@ -78,6 +79,27 @@ Used to manage host level sysctl tunables. Ex:: net/ipv4/ip_forward: 1 net/ipv6/conf/all/forwarding: 1 +limits +^^^^^^ + +Used to manage host level limits. Ex:: + + conf: + limits: + nofile: + domain: 'root' + type: 'soft' + item: 'nofile' + value: '101' + core_dump: + domain: '0:' + type: 'hard' + item: 'core' + value: 0 + +Previous values of newly set limits are backed up to /var/divingbell/limits + + mounts ^^^^^^ @@ -256,6 +278,18 @@ Caveats: "another_label" would take precedence and be applied to nodes that contained both of the defined labels. +Dev Environment with Vagrant +---------------------------- +The point of Dev env to prepare working environment for development. + +Vagrantfile allows to run on working copy with modifications +e.g. to 020-test script. The approach is to setup Gate test +but do not delete the pods and other stuff. You have: + +1. test run of previous tests and their results +2. your changes from working tree are applied smoothly +3. your not committed test runs in prepared env + Recorded Demo ------------- diff --git a/tools/gate/scripts/020-test-divingbell.sh b/tools/gate/scripts/020-test-divingbell.sh index c94adb6..9042ec6 100755 --- a/tools/gate/scripts/020-test-divingbell.sh +++ b/tools/gate/scripts/020-test-divingbell.sh @@ -57,7 +57,7 @@ for line in ${nic_info}; do fi if [ "${physical_nic}" = 'true' ] && [[ ${line} = *'logical name'* ]]; then DEVICE="$(echo "${line}" | cut -d':' -f2 | tr -d '[:space:]')" - echo "Found deivce: '${DEVICE}' to use for ethtool testing" + echo "Found device: '${DEVICE}' to use for ethtool testing" break fi done @@ -99,6 +99,7 @@ _teardown_systemd(){ clean_persistent_files(){ sudo rm -r /var/${NAME} >& /dev/null || true sudo rm -r /etc/sysctl.d/60-${NAME}-* >& /dev/null || true + sudo rm -r /etc/security/limits.d/60-${NAME}-* >& /dev/null || true _teardown_systemd ${MOUNTS_PATH1} mount _teardown_systemd ${MOUNTS_PATH2} mount _teardown_systemd ${MOUNTS_PATH3} mount @@ -312,6 +313,38 @@ test_sysctl(){ echo '[SUCCESS] sysctl test5 passed successfully' >> "${TEST_RESULTS}" } +_test_limits_value(){ + local limit=${1} + local domain=${2} + local type=${3} + local item=${4} + local value=${5} + test "$(cat /etc/security/limits.d/60-${NAME}-${limit}.conf)" = \ + "$domain $type $item $value" +} + +test_limits(){ + local overrides_yaml=${LOGS_SUBDIR}/${FUNCNAME}.yaml + echo "conf: + limits: + limit1: + domain: root + type: hard + item: core + value: 0 + limit2: + domain: '0:' + type: soft + item: nofile + value: 101" > "${overrides_yaml}" + echo $(cat ${overrides_yaml}) + install_base "--values=${overrides_yaml}" + get_container_status limits + _test_limits_value limit1 root hard core 0 + _test_limits_value limit2 '0:' soft nofile 101 + echo "[SUCCESS] test range loop for limits passed successfully" >> "${TEST_RESULTS}" +} + _test_if_mounted_positive(){ mountpoint "${1}" || (echo "Expect ${1} to be mounted, but was not"; exit 1) df -h | grep "${1}" | grep "${2}" || @@ -815,9 +848,9 @@ test_overrides(){ # Compare against expected number of generated daemonsets daemonset_count="$(echo "${tc_output}" | grep 'kind: DaemonSet' | wc -l)" - if [ "${daemonset_count}" != "12" ]; then + if [ "${daemonset_count}" != "13" ]; then echo '[FAILURE] overrides test 1 failed' >> "${TEST_RESULTS}" - echo "Expected 12 daemonsets; got '${daemonset_count}'" >> "${TEST_RESULTS}" + echo "Expected 13 daemonsets; got '${daemonset_count}'" >> "${TEST_RESULTS}" exit 1 else echo '[SUCCESS] overrides test 1 passed successfully' >> "${TEST_RESULTS}" @@ -995,13 +1028,14 @@ init_default_state # run tests install_base test_sysctl +test_limits test_mounts test_ethtool test_uamlite purge_containers test_overrides -# retore initial state +# restore initial state init_default_state echo "All tests pass for ${NAME}"