Skip to content

Commit a6ed570

Browse files
sfc-gh-jmckulkajmckulk
authored andcommitted
Run Shellcheck on instance reload command
Shellcheck was being run on the reload command for pgBackRest. During the auto-grow work, we noticed that the reload command for the instance was not being checked. Since we had to update the auto-grow bash logic to pass Shellcheck for pgBackRest, it seemed appropriate to enable the checks for the instance as well.
1 parent af97e79 commit a6ed570

File tree

3 files changed

+80
-22
lines changed

3 files changed

+80
-22
lines changed

internal/postgres/config.go

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -298,13 +298,24 @@ func reloadCommand(
298298
// descriptor gets closed and reopened to use the builtin `[ -nt` to check
299299
// mtimes.
300300
// - https://unix.stackexchange.com/a/407383
301+
//
302+
// In the manageAutogrowAnnotation function below, df is used to return the
303+
// relevant volume size in Mebibytes. The 'read' variable gets the value from
304+
// the '1M-blocks' output (second column) and the 'use' variable gets the value
305+
// from the 'Use%' column (fifth column). This value is grabbed after stripping
306+
// out the column headers (before the '\n') and then getting the respective
307+
// value delimited by the white spaces by using the 'read -r' command.
308+
// The underscores (_) discard fields and the variables store them. This allows
309+
// for selective parsing of the provided lines. The percent value is stripped of
310+
// the '%' and then used to determine if a expansion should be triggered by
311+
// setting the calculated volume size using the 'size' variable.
301312
script := fmt.Sprintf(`
302313
# Parameters for curl when managing autogrow annotation.
303314
APISERVER="https://kubernetes.default.svc"
304315
SERVICEACCOUNT="/var/run/secrets/kubernetes.io/serviceaccount"
305-
NAMESPACE=$(cat ${SERVICEACCOUNT}/namespace)
306-
TOKEN=$(cat ${SERVICEACCOUNT}/token)
307-
CACERT=${SERVICEACCOUNT}/ca.crt
316+
NAMESPACE=$(cat "${SERVICEACCOUNT}/namespace")
317+
TOKEN=$(cat "${SERVICEACCOUNT}/token")
318+
CACERT="${SERVICEACCOUNT}/ca.crt"
308319
309320
# Manage autogrow annotation.
310321
# Return size in Mebibytes.
@@ -313,27 +324,29 @@ manageAutogrowAnnotation() {
313324
local trigger=$2
314325
local maxGrow=$3
315326
316-
size=$(df --block-size=M /"${volume}" | awk 'FNR == 2 {print $2}')
317-
use=$(df /"${volume}" | awk 'FNR == 2 {print $5}')
327+
size=$(df --block-size=M /"${volume}")
328+
read -r _ size _ <<< "${size#*$'\n'}"
329+
use=$(df /"${volume}")
330+
read -r _ _ _ _ use _ <<< "${use#*$'\n'}"
318331
sizeInt="${size//M/}"
319332
# Use the sed punctuation class, because the shell will not accept the percent sign in an expansion.
320-
useInt=$(echo $use | sed 's/[[:punct:]]//g')
333+
useInt=${use//[[:punct:]]/}
321334
triggerExpansion="$((useInt > trigger))"
322-
if [[ $triggerExpansion -eq 1 ]]; then
335+
if [[ ${triggerExpansion} -eq 1 ]]; then
323336
newSize="$(((sizeInt / 2)+sizeInt))"
324337
# Only compare with maxGrow if it is set (not empty)
325-
if [[ -n "$maxGrow" ]]; then
338+
if [[ -n "${maxGrow}" ]]; then
326339
# check to see how much we would normally grow
327340
sizeDiff=$((newSize - sizeInt))
328341
329342
# Compare the size difference to the maxGrow; if it is greater, cap it to maxGrow
330-
if [[ $sizeDiff -gt $maxGrow ]]; then
343+
if [[ ${sizeDiff} -gt ${maxGrow} ]]; then
331344
newSize=$((sizeInt + maxGrow))
332345
fi
333346
fi
334347
newSizeMi="${newSize}Mi"
335-
d='[{"op": "add", "path": "/metadata/annotations/suggested-'"${volume}"'-pvc-size", "value": "'"$newSizeMi"'"}]'
336-
curl --cacert ${CACERT} --header "Authorization: Bearer ${TOKEN}" -XPATCH "${APISERVER}/api/v1/namespaces/${NAMESPACE}/pods/${HOSTNAME}?fieldManager=kubectl-annotate" -H "Content-Type: application/json-patch+json" --data "$d"
348+
d='[{"op": "add", "path": "/metadata/annotations/suggested-'"${volume}"'-pvc-size", "value": "'"${newSizeMi}"'"}]'
349+
curl --cacert "${CACERT}" --header "Authorization: Bearer ${TOKEN}" -XPATCH "${APISERVER}/api/v1/namespaces/${NAMESPACE}/pods/${HOSTNAME}?fieldManager=kubectl-annotate" -H "Content-Type: application/json-patch+json" --data "${d}"
337350
fi
338351
}
339352

internal/postgres/config_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@ import (
1818
"time"
1919

2020
"gotest.tools/v3/assert"
21+
"k8s.io/apimachinery/pkg/api/resource"
2122
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2223
"sigs.k8s.io/yaml"
2324

25+
"github.com/crunchydata/postgres-operator/internal/initialize"
2426
"github.com/crunchydata/postgres-operator/internal/testing/cmp"
2527
"github.com/crunchydata/postgres-operator/internal/testing/require"
2628
"github.com/crunchydata/postgres-operator/pkg/apis/postgres-operator.crunchydata.com/v1beta1"
@@ -600,3 +602,44 @@ EOF
600602
chmod +x /tmp/pg_rewind_tde.sh`))
601603
})
602604
}
605+
606+
func TestReloadCommand(t *testing.T) {
607+
shellcheck := require.ShellCheck(t)
608+
609+
pgdataSize := resource.MustParse("1Gi")
610+
pgwalSize := resource.MustParse("2Gi")
611+
612+
command := reloadCommand(
613+
"some-name",
614+
&v1beta1.VolumeClaimSpecWithAutoGrow{
615+
AutoGrow: &v1beta1.AutoGrowSpec{
616+
Trigger: initialize.Int32(10),
617+
MaxGrow: &pgdataSize,
618+
},
619+
},
620+
&v1beta1.VolumeClaimSpecWithAutoGrow{
621+
AutoGrow: &v1beta1.AutoGrowSpec{
622+
Trigger: initialize.Int32(20),
623+
MaxGrow: &pgwalSize,
624+
},
625+
},
626+
)
627+
628+
// Expect a bash command with an inline script.
629+
assert.DeepEqual(t, command[:3], []string{"bash", "-ceu", "--"})
630+
assert.Assert(t, len(command) > 3)
631+
632+
// Write out that inline script.
633+
dir := t.TempDir()
634+
file := filepath.Join(dir, "script.bash")
635+
assert.NilError(t, os.WriteFile(file, []byte(command[3]), 0o600))
636+
637+
// Expect shellcheck to be happy.
638+
cmd := exec.CommandContext(t.Context(), shellcheck, "--enable=all", file)
639+
output, err := cmd.CombinedOutput()
640+
assert.NilError(t, err, "%q\n%s", cmd.Args, output)
641+
642+
assert.Assert(t, cmp.Contains(command[3], "manageAutogrowAnnotation \"pgdata\" \"10\" \"1024\""))
643+
assert.Assert(t, cmp.Contains(command[3], "manageAutogrowAnnotation \"pgwal\" \"20\" \"2048\""))
644+
645+
}

internal/postgres/reconcile_test.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,9 @@ containers:
173173
# Parameters for curl when managing autogrow annotation.
174174
APISERVER="https://kubernetes.default.svc"
175175
SERVICEACCOUNT="/var/run/secrets/kubernetes.io/serviceaccount"
176-
NAMESPACE=$(cat ${SERVICEACCOUNT}/namespace)
177-
TOKEN=$(cat ${SERVICEACCOUNT}/token)
178-
CACERT=${SERVICEACCOUNT}/ca.crt
176+
NAMESPACE=$(cat "${SERVICEACCOUNT}/namespace")
177+
TOKEN=$(cat "${SERVICEACCOUNT}/token")
178+
CACERT="${SERVICEACCOUNT}/ca.crt"
179179
180180
# Manage autogrow annotation.
181181
# Return size in Mebibytes.
@@ -184,27 +184,29 @@ containers:
184184
local trigger=$2
185185
local maxGrow=$3
186186
187-
size=$(df --block-size=M /"${volume}" | awk 'FNR == 2 {print $2}')
188-
use=$(df /"${volume}" | awk 'FNR == 2 {print $5}')
187+
size=$(df --block-size=M /"${volume}")
188+
read -r _ size _ <<< "${size#*$'\n'}"
189+
use=$(df /"${volume}")
190+
read -r _ _ _ _ use _ <<< "${use#*$'\n'}"
189191
sizeInt="${size//M/}"
190192
# Use the sed punctuation class, because the shell will not accept the percent sign in an expansion.
191-
useInt=$(echo $use | sed 's/[[:punct:]]//g')
193+
useInt=${use//[[:punct:]]/}
192194
triggerExpansion="$((useInt > trigger))"
193-
if [[ $triggerExpansion -eq 1 ]]; then
195+
if [[ ${triggerExpansion} -eq 1 ]]; then
194196
newSize="$(((sizeInt / 2)+sizeInt))"
195197
# Only compare with maxGrow if it is set (not empty)
196-
if [[ -n "$maxGrow" ]]; then
198+
if [[ -n "${maxGrow}" ]]; then
197199
# check to see how much we would normally grow
198200
sizeDiff=$((newSize - sizeInt))
199201
200202
# Compare the size difference to the maxGrow; if it is greater, cap it to maxGrow
201-
if [[ $sizeDiff -gt $maxGrow ]]; then
203+
if [[ ${sizeDiff} -gt ${maxGrow} ]]; then
202204
newSize=$((sizeInt + maxGrow))
203205
fi
204206
fi
205207
newSizeMi="${newSize}Mi"
206-
d='[{"op": "add", "path": "/metadata/annotations/suggested-'"${volume}"'-pvc-size", "value": "'"$newSizeMi"'"}]'
207-
curl --cacert ${CACERT} --header "Authorization: Bearer ${TOKEN}" -XPATCH "${APISERVER}/api/v1/namespaces/${NAMESPACE}/pods/${HOSTNAME}?fieldManager=kubectl-annotate" -H "Content-Type: application/json-patch+json" --data "$d"
208+
d='[{"op": "add", "path": "/metadata/annotations/suggested-'"${volume}"'-pvc-size", "value": "'"${newSizeMi}"'"}]'
209+
curl --cacert "${CACERT}" --header "Authorization: Bearer ${TOKEN}" -XPATCH "${APISERVER}/api/v1/namespaces/${NAMESPACE}/pods/${HOSTNAME}?fieldManager=kubectl-annotate" -H "Content-Type: application/json-patch+json" --data "${d}"
208210
fi
209211
}
210212

0 commit comments

Comments
 (0)