feat: support sidecar initContainers

[ihor-hrytskiv]

This is a proposal to solve the issue #381. If you approve this design, or if we choose another, I will write tests to cover the code.

Sidecar containers are a new type of containers that start among the Init containers, run through the lifecycle of the Pod and don’t block pod termination. Kubelet makes a best effort to keep them alive and running while other containers are running.

• Tests pass
• Appropriate changes to README are included in PR

[jackwotherspoon]

Thanks @ihor-hrytskiv for the PR and contribution! 😄

@hessjcg is OOO this week, but will review this first thing next week when he is back

[hessjcg]

Hi, nice work, @ihor-hrytskiv. Thank you for contributing.

I would like to suggest a bigger change: Instead of creating a new option in the custom resource for sidecar container, the operator should check the Kubernetes API version. The operator always use InitContainer sidecars if they are supported.

I will review the code in more detail over the next few days. I think there may an issue with the logic to maintain the list of ports in use. I'll more review comments soon.

-Jonathan

[ihor-hrytskiv]

Hi, nice work, @ihor-hrytskiv. Thank you for contributing.

I would like to suggest a bigger change: Instead of creating a new option in the custom resource for sidecar container, the operator should check the Kubernetes API version. The operator always use InitContainer sidecars if they are supported.

I will review the code in more detail over the next few days. I think there may an issue with the logic to maintain the list of ports in use. I'll more review comments soon.

-Jonathan

Hi @hessjcg, thanks, I'll try to add this functionality as soon as possible

[ihor-hrytskiv]

Hi @hessjcg, take a look please

[hessjcg]

Nice work. I pushed a commit that updates the integration tests so that they run on the k8s versions 1.28 and latest. I'm going to work on running the end-to-end tests now.

[hessjcg]

Instead of hardcoding the envtest k8s version here, let's add new make target go_test_k8s_1_28 which will run the integration tests using K8s 1.28.x.

[hessjcg]

Good cleanup. Thank you.

[jackwotherspoon]

Nice work. I pushed a commit that updates the integration tests so that they run on the k8s versions 1.28 and latest. I'm going to work on running the end-to-end tests now.

@hessjcg We no longer run integration tests on forks due to the security issues with the pull_request_target GitHub actions trigger we used to use.

So there are two options here:

1. Merge the PR and monitor integrations tests on the continuous build on main closely, revert or send follow up PR to fix tests if the builds fails.
2. Copy the forked branch to your own internal branch and have the tests run on it and then merge this PR and delete your copy branch,

[hessjcg]

@jackwotherspoon, Good point. I'm going to run the e2e tests in my dev environment, and if they pass, then I will merge and monitor the e2e tests on main. I would like to build a way to run e2e tests on PRs again, but that will have to wait.

[hessjcg]

E2E tests passed: E2E Test Github Action. I'm going to approve and merge this PR.

[utmaks]

After upgrading to version 1.6.0, I got a lot of runtime validation errors on startup (Startup probe failed: Get "http://10.210.0.89:15020/app-health/csql-apps-apps/startupz": dial tcp 10.210.0.89:15020: connect: connection refused) and can't get back the previous behavior (I need version 1.6.0 because of the minSigtermDelay parameter). At the same time, the permissions of the service accounts were not changed in any way. Old versions of pods work in the database. Is everything exactly working here?

I'm using istio and maybe this is the case, since the deployment is modified several times. Especially since csql tries to start the first istio-init container, before istio-init. The new approach doesn't work like that right away.

UPD.

My hypothesis turned out to be correct. After disabling Istio, the proxy container was up properly. I think we should let the user choose the mode of operation himself, as it was in the first commit (sidecarType) ...

[jackwotherspoon]

@utmaks Thanks for raising this! 😄

Do you mind creating a new issue on this repo so we can get this addressed and fixed.

Thanks in advance.

[utmaks]

#641