Skip to content

Commit 93ead3d

Browse files
Swapnanil-GuptaSwapnanil-Gupta
authored
feat: enable healthchecks (#1655)
* deps: bump nerdctl, enable healthchecks Signed-off-by: Swapnanil Gupta <[email protected]> * update github.com/containerd/cgroups package to v3 Signed-off-by: Swapnanil Gupta <[email protected]> * bump common-tests so healthchecks are skipped on nerdctl<2.2.1 Signed-off-by: Swapnanil Gupta <[email protected]> * bump finch-core which internally rolls back finch daemon to 0.20.0 from 0.21.0 Signed-off-by: Swapnanil Gupta <[email protected]> * Changes: - bump finch-core again so that we revert back to finch-daemon 0.21 - update AL tests workflow to overwrite nerdctl version in CI Signed-off-by: Swapnanil Gupta <[email protected]> * update expected dependency versions in vm serial tests Signed-off-by: Swapnanil Gupta <[email protected]> * fix handling of flags that can have negative integer values Signed-off-by: Swapnanil-Gupta <[email protected]> * fix inNumericArg function and add unit tests for handleFlagArg function Signed-off-by: Swapnanil-Gupta <[email protected]> --------- Signed-off-by: Swapnanil Gupta <[email protected]> Signed-off-by: Swapnanil-Gupta <[email protected]>
1 parent b658c43 commit 93ead3d

File tree

9 files changed

+322
-104
lines changed

9 files changed

+322
-104
lines changed

‎.github/workflows/e2e-linux.yaml‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ jobs:
8686
sudo systemctl daemon-reload
8787
sudo rm -rf /etc/finch
8888
sudo rm -rf /var/lib/finch
89+
sudo rm -rf /var/lib/nerdctl
8990
sudo rm -rf /var/lib/containerd
9091
sudo rm -rf /var/soci-snapshotter
9192
sudo rm -rf ./_output
@@ -99,6 +100,18 @@ jobs:
99100
- name: Install Finch
100101
run: |
101102
sudo rpm -i ./_output/packages/$(ls -t ./_output/packages/ | grep runfinch-finch | head -1)
103+
104+
# TODO: remove this once nerdctl v2.2.1 is released in AL
105+
ARCH=$(uname -m)
106+
case $ARCH in
107+
x86_64) NERDCTL_ARCH="amd64" ;;
108+
aarch64) NERDCTL_ARCH="arm64" ;;
109+
esac
110+
wget "https://github.com/containerd/nerdctl/releases/download/v2.2.1/nerdctl-2.2.1-linux-${NERDCTL_ARCH}.tar.gz"
111+
sudo tar Cxzvf /usr/local/bin nerdctl-2.2.1-linux-${NERDCTL_ARCH}.tar.gz
112+
rm nerdctl-2.2.1-linux-${NERDCTL_ARCH}.tar.gz
113+
/usr/local/bin/nerdctl --version
114+
102115
sudo systemctl daemon-reload
103116
sudo systemctl restart containerd.service
104117
sudo systemctl restart finch.socket

‎cmd/finch/nerdctl_remote.go‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,11 @@ func (nc *nerdctlCommand) handleFlagArg(arg string, nextArg string) (bool, strin
507507
// long flag concatenated to value by '=': --<long_flag>="<value>"
508508
skip = false
509509
flagKey, flagVal, _ = strings.Cut(arg, "=")
510-
case strings.HasPrefix(arg, "--") && !strings.HasPrefix(nextArg, "-"):
511-
// long flag followed by a value: --<long_flag> "<value>"
510+
case strings.HasPrefix(arg, "--") && isNumericArg(nextArg),
511+
strings.HasPrefix(arg, "--") && !strings.HasPrefix(nextArg, "-"):
512+
// long flag followed by a value (including a negative number): --<long_flag> "<value>".
513+
// the isNumeric check is needed because the value can be a negative number.
514+
// for example, in our health check tests where we pass --health-retries -5 or --health-timeout -5s
512515
skip = true
513516
flagKey = arg
514517
flagVal = nextArg
@@ -522,8 +525,9 @@ func (nc *nerdctlCommand) handleFlagArg(arg string, nextArg string) (bool, strin
522525
skip = false
523526
flagKey = arg[:2]
524527
flagVal = arg[2:]
525-
case strings.HasPrefix(arg, "-") && len(arg) == 2 && !strings.HasPrefix(nextArg, "-"):
526-
// short flag followed by a value: -? "<value>" or -? <value>
528+
case strings.HasPrefix(arg, "-") && len(arg) == 2 && isNumericArg(nextArg),
529+
strings.HasPrefix(arg, "-") && len(arg) == 2 && !strings.HasPrefix(nextArg, "-"):
530+
// short flag followed by a value (including a negative number): -? "<value>" or -? <value>
527531
skip = true
528532
flagKey = arg
529533
flagVal = nextArg
@@ -613,3 +617,29 @@ func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 s
613617
}
614618
return skip, envs, nil
615619
}
620+
621+
// isNumericArg returns whether the passed argument is a numeric argument or not.
622+
// For example, it returns true for cases like 5, -5, 5s and -5s.
623+
func isNumericArg(arg string) bool {
624+
if arg == "" {
625+
return false
626+
}
627+
// handle the case where the arg is a negative number followed by a char
628+
// for example: --health-timeout -5s
629+
if arg[0] == '-' && len(arg) > 1 {
630+
for i := 1; i < len(arg); i++ {
631+
if arg[i] < '0' || arg[i] > '9' {
632+
return i > 1
633+
}
634+
}
635+
return true
636+
}
637+
// handle the case where the arg is a positive number followed by a char
638+
// for example: --health-timeout -5s
639+
for i := 0; i < len(arg); i++ {
640+
if arg[i] < '0' || arg[i] > '9' {
641+
return i > 0
642+
}
643+
}
644+
return true
645+
}

‎cmd/finch/nerdctl_remote_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,3 +132,177 @@ func TestNerdctlCommand_withVMErrors(t *testing.T) {
132132
})
133133
}
134134
}
135+
136+
func TestHandleFlagArg(t *testing.T) {
137+
t.Parallel()
138+
testCases := []struct {
139+
inputArgs []string
140+
expectedArgs []string
141+
}{
142+
{
143+
inputArgs: []string{"--flag", "value"},
144+
expectedArgs: []string{"--flag", "value"},
145+
},
146+
{
147+
inputArgs: []string{"--flag=value", ""},
148+
expectedArgs: []string{"--flag", "value"},
149+
},
150+
{
151+
inputArgs: []string{"--flag=\"value\"", ""},
152+
expectedArgs: []string{"--flag", "\"value\""},
153+
},
154+
{
155+
inputArgs: []string{"--flag", "10"},
156+
expectedArgs: []string{"--flag", "10"},
157+
},
158+
{
159+
inputArgs: []string{"--flag", "-10"},
160+
expectedArgs: []string{"--flag", "-10"},
161+
},
162+
{
163+
inputArgs: []string{"--flag", "10s"},
164+
expectedArgs: []string{"--flag", "10s"},
165+
},
166+
{
167+
inputArgs: []string{"--flag", "-10s"},
168+
expectedArgs: []string{"--flag", "-10s"},
169+
},
170+
{
171+
inputArgs: []string{"--flag=10", ""},
172+
expectedArgs: []string{"--flag", "10"},
173+
},
174+
{
175+
inputArgs: []string{"--flag=10s", ""},
176+
expectedArgs: []string{"--flag", "10s"},
177+
},
178+
{
179+
inputArgs: []string{"--flag=-10", ""},
180+
expectedArgs: []string{"--flag", "-10"},
181+
},
182+
{
183+
inputArgs: []string{"--flag=-10s", ""},
184+
expectedArgs: []string{"--flag", "-10s"},
185+
},
186+
{
187+
inputArgs: []string{"--flag=\"10\"", ""},
188+
expectedArgs: []string{"--flag", "\"10\""},
189+
},
190+
{
191+
inputArgs: []string{"--flag=\"-10\"", ""},
192+
expectedArgs: []string{"--flag", "\"-10\""},
193+
},
194+
{
195+
inputArgs: []string{"--flag=\"10s\"", ""},
196+
expectedArgs: []string{"--flag", "\"10s\""},
197+
},
198+
{
199+
inputArgs: []string{"--flag=\"-10s\"", ""},
200+
expectedArgs: []string{"--flag", "\"-10s\""},
201+
},
202+
{
203+
inputArgs: []string{"-f", "value"},
204+
expectedArgs: []string{"-f", "value"},
205+
},
206+
{
207+
inputArgs: []string{"-f=value", ""},
208+
expectedArgs: []string{"-f", "value"},
209+
},
210+
{
211+
inputArgs: []string{"-f=\"value\"", ""},
212+
expectedArgs: []string{"-f", "\"value\""},
213+
},
214+
{
215+
inputArgs: []string{"-f", "10s"},
216+
expectedArgs: []string{"-f", "10s"},
217+
},
218+
{
219+
inputArgs: []string{"-f", "-10s"},
220+
expectedArgs: []string{"-f", "-10s"},
221+
},
222+
{
223+
inputArgs: []string{"-f=10", ""},
224+
expectedArgs: []string{"-f", "10"},
225+
},
226+
{
227+
inputArgs: []string{"-f=10s", ""},
228+
expectedArgs: []string{"-f", "10s"},
229+
},
230+
{
231+
inputArgs: []string{"-f=-10", ""},
232+
expectedArgs: []string{"-f", "-10"},
233+
},
234+
{
235+
inputArgs: []string{"-f=-10s", ""},
236+
expectedArgs: []string{"-f", "-10s"},
237+
},
238+
{
239+
inputArgs: []string{"-f=\"10\"", ""},
240+
expectedArgs: []string{"-f", "\"10\""},
241+
},
242+
{
243+
inputArgs: []string{"-f=\"10s\"", ""},
244+
expectedArgs: []string{"-f", "\"10s\""},
245+
},
246+
{
247+
inputArgs: []string{"-f=\"-10\"", ""},
248+
expectedArgs: []string{"-f", "\"-10\""},
249+
},
250+
{
251+
inputArgs: []string{"-f=\"-10s\"", ""},
252+
expectedArgs: []string{"-f", "\"-10s\""},
253+
},
254+
{
255+
inputArgs: []string{"-f10", ""},
256+
expectedArgs: []string{"-f", "10"},
257+
},
258+
}
259+
for _, tc := range testCases {
260+
ctrl := gomock.NewController(t)
261+
ecc := mocks.NewCommandCreator(ctrl)
262+
ncc := mocks.NewNerdctlCmdCreator(ctrl)
263+
ncsd := mocks.NewNerdctlCommandSystemDeps(ctrl)
264+
logger := mocks.NewLogger(ctrl)
265+
fs := afero.NewMemMapFs()
266+
nc := newNerdctlCommand(ncc, ecc, ncsd, logger, fs, nil)
267+
// the first return value is never used
268+
_, flagKey, flagVal := nc.handleFlagArg(tc.inputArgs[0], tc.inputArgs[1])
269+
assert.Equal(t, flagKey, tc.expectedArgs[0])
270+
assert.Equal(t, flagVal, tc.expectedArgs[1])
271+
}
272+
}
273+
274+
func TestIsNumericArg(t *testing.T) {
275+
t.Parallel()
276+
testCases := []struct {
277+
arg string
278+
expected bool
279+
}{
280+
{
281+
arg: "5",
282+
expected: true,
283+
},
284+
{
285+
arg: "5s",
286+
expected: true,
287+
},
288+
{
289+
arg: "-5",
290+
expected: true,
291+
},
292+
{
293+
arg: "-5s",
294+
expected: true,
295+
},
296+
{
297+
arg: "abc",
298+
expected: false,
299+
},
300+
{
301+
arg: "-abc",
302+
expected: false,
303+
},
304+
}
305+
for _, tc := range testCases {
306+
assert.Equal(t, tc.expected, isNumericArg(tc.arg))
307+
}
308+
}

‎deps/finch-core‎

‎e2e/container/container_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func TestContainer(t *testing.T) {
112112
tests.NetworkInspect(o)
113113
tests.NetworkLs(o)
114114
tests.NetworkRm(o)
115+
tests.HealthCheck(o)
115116
testCosign(o)
116117
testCrossPlatformRun(o)
117118
})

‎e2e/container/container_test_linux.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
package container
88

99
import (
10-
"github.com/containerd/cgroups"
10+
"github.com/containerd/cgroups/v3"
1111
"github.com/runfinch/common-tests/tests"
1212
)
1313

‎e2e/vm/version_remote_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ import (
1818
"github.com/runfinch/finch/pkg/version"
1919
)
2020

21+
// TODO: read this from finch-core/deps.
2122
const (
22-
nerdctlVersion = "v2.1.3"
23-
buildKitVersion = "v0.23.2"
24-
containerdVersion = "v2.1.3"
23+
nerdctlVersion = "v2.2.1"
24+
buildKitVersion = "v0.26.3"
25+
containerdVersion = "v2.2.1"
2526
runcVersion = "1.3.3"
2627
)
2728

0 commit comments

Comments
 (0)