gha: kata-deploy: Revert containerd config break

[stevenhorsman]

After kata-deploy has installed, check containerd is still in active state, to ensure that we didn't corrupt the containerd config during kata-deploy's edits

Fixes: #8678

[stevenhorsman]

@Amulyam24 - FYI the ppc64le checks are failing with:

Step 1/23 : FROM ubuntu:20.04
toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
make[1]: *** [tools/packaging/kata-deploy/local-build/Makefile:75: kernel-tarball-build] Error 1
make: *** [tools/packaging/kata-deploy/local-build/Makefile:111: kernel-tarball] Error 2

I'm guessing that we need to login to docker hub on the self-hosted node as the anonymous rate limit is pretty low?

[beraldoleal]

LGTM, thanks @stevenhorsman. My only question is if this is indeed fixing #8678 as described at the commit message. Unless I'm missing something, this is just adding a test, not fixing the actual problem, right?

[stevenhorsman]

LGTM, thanks @stevenhorsman. My only question is if this is indeed fixing #8678 as described at the commit message. Unless I'm missing something, this is just adding a test, not fixing the actual problem, right?

Correct - this is just the first stage of the PR, assuming it works (and the test fails), we'll have a way to test the issue and I plan to revert part of the tomlq PR and check that fixes things. I'll add do-not-merge to this to show it's not completed.

[stevenhorsman]

/test

[stevenhorsman]

Ok, I think this containerd test approach works for baremetal systems, but not a other clusters as we can't necessarily get in an access their containerd. I can try using kata-debug for this process though...

[stevenhorsman]

I didn't get kata-debug working reliably, so just using kubectl with custom columns to extract status and containerruntimeversion. I'll also temporarily dropped the fix, so I can check that this test is correctly failing.

[stevenhorsman]

In the previous run, I got some of the deploy tests to fail as expected, but more of them were still passing, which doesn't really make sense, so I've upped the sleep to see if that gives enough time for the containerd config changes to take effect.

[stevenhorsman]

/test

[stevenhorsman]

As I shared in confidential-containers/operator#305 (comment) - I'm not super happy with this change and think the testing could do with some wider input, but the revert of using tomlq resolves the problem of kata-deploy corrupting containerd's config, so I think it's good enough to merge and unblock people until January when more people are back to discuss this.

[stevenhorsman]

/test

[stevenhorsman]

/test

[stevenhorsman]

/test

[stevenhorsman]

/test

[gkurz]

LGTM. Thanks @stevenhorsman !

[fidencio]

Folks, I don't like the commit being reverted, but I must say it's what it's at this point.

As I'm working Today, I've decided to poke Steve and get to the root cause of the issue, and here's the full explanation.
tomlq relies on jq in order to do the parsing / editing of the file, and it's done in this specific code path

            elif input_format == "toml":
                import tomlkit

                for input_stream in input_streams:
                    json.dump(tomlkit.load(input_stream), jq.stdin, cls=JSONDateTimeEncoder)  # type: ignore
                    jq.stdin.write("\n")  # type: ignore

The interesting part to understand here is that jq is an invocation of the jq command, as shown here

        jq = subprocess.Popen(
            ["jq"] + list(jq_args),
            stdin=subprocess.PIPE,
            stdout=subprocess.PIPE if converting_output else None,
            close_fds=False,
            universal_newlines=True,
        )

Okay, with this in mind, we can simplify a whole lot the test case.
Let's test with echo '{"foo": 1.0}' | jq .foo.

/ # jq --version
jq-1.6
/ # echo '{"foo": 1.0}' | jq .foo
1

Bingo, we can reproduce the issue we've faced and we know that the version of jq being used is 1.6 When taking a look at jq's GitHub page, I've noticed a newer release, 1.7, and decided to give it a try before hunting this issue down.

⋊> Downloads ./jq-linux-amd64 --version
jq-1.7
⋊> Downloads echo '{"foo": 1.0}' | jq .foo
1.0

And, hey, it works as expected.

So, the actual solution for the issue should be:

• Re-revert the reverted commit
• Bump the jq version used in our daemonset
• Profit

I will open a PR with this soon enough, and this will allow us to move on and get the snapshotter work merged, unblocking others depending on this one.