config: add --action-env cli option

[rst0git]

In some use cases, action scripts might require user-specified environment variables to be passed. For example, when creating a checkpoint of a Pod, we would like to be able to pass the container name and pod name to the action script. To enable this functionality, this pull request adds a new CLI option --action-env that takes as input a string in the following format <NAME>=<VALUE> . This option can be used multiple times.

Example:

criu dump --action-script /foo/bar/script.sh  --action-env "FOO1=BAR1"  --action-env "FOO2=BAR2" ...

[adrianreber]

CI is broken because of it, but this is a case where you could extend the unit tests to make sure this use case is also tested.

Every time strings are parsed a unit test is probably a useful addition. Especially error paths can be tested during unit tests.

There is also some command line option and configuration file testing which can be enhanced to ensure that the new options are tested.

Overall I think this sounds useful.

For example, when creating a checkpoint of a Pod, we would like to be able to pass the container name and pod name to the action script.

Not sure about this argument. How is the container and pod name relevant on the CRIU level which only deals with PIDs? Maybe you can elaborate a bit more how this can be useful.

[codecov-commenter]

Codecov Report

Patch coverage: 75.47% and no project coverage change.

Comparison is base (b845846) 70.36% compared to head (48f828c) 70.36%.

❗ Current head 48f828c differs from pull request most recent head e8dc491. Consider uploading reports for the commit e8dc491 to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##           criu-dev    #2136   +/-   ##
=========================================
  Coverage     70.36%   70.36%           
=========================================
  Files           133      133           
  Lines         33894    33946   +52     
=========================================
+ Hits          23848    23887   +39     
- Misses        10046    10059   +13     

Impacted Files	Coverage Δ
criu/crtools.c	67.03% <ø> (ø)
criu/unittest/mock.c	8.16% <0.00%> (-0.35%)	⬇️
criu/action-scripts.c	77.17% <63.15%> (-3.91%)	⬇️
criu/config.c	82.11% <82.60%> (+0.02%)	⬆️
criu/unittest/unit.c	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rst0git]

Thanks Adrian! I've updated the unit tests with a new test for the --action-env option

How is the container and pod name relevant on the CRIU level which only deals with PIDs?

When a distributed application that consist of multiple containers is running across different nodes, each container could have dependency on one or more others containers. In order to synchronise and coordinate the checkpointing/restoring of multiple containers we can use action-scripts. However, the information currently provided to an action script is insufficient to identify the container that is being checkpointed/restored.

The --action-env option allows to pass arbitrary information to the action script. In particular, the container name and pod name can be used to coordinate at CRIU-level the checkpointing / restoring across multiple nodes.

[adrianreber]

Your changes are partially not covered by tests. Adding something to https://github.com/checkpoint-restore/criu/blob/criu-dev/test/others/config-file/run.sh would help.

[rst0git]

I've updated the pull request with a test that covers the action-script functionality.

[adrianreber]

Looks good. Very good test coverage and the code looks correct regarding memory management.

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[avagin]

Can we execute criu with all these variables and then action scripts will inherit them?

[rst0git]

Can we execute criu with all these variables and then action scripts will inherit them?

Yes, these variables don't affect how criu works. We are currently working on a tool called criu-coordinator. It is invoked as an action script that coordinates the dump/restore process of multiple criu instances to create a consistent global checkpoint and to enable rollback-recovery of distributed applications. This is similar to how dmtcp_coordinator would synchronise the checkpoint/restore sequence but instead of "barriers" we use the "hooks" available in CRIU.

For example, a configuration file that would enable this functionality looks something like the following:

$ cat looper-1.conf 
action-script /usr/local/bin/criu-coordinator
action-env CR_ADDR=127.0.0.1
action-env CR_PORT=8080
action-env CR_ID=looper-looper-1
action-env CR_DEPS=looper-looper-2:looper-looper-3

where CR_ADDR / CR_PORT are address and port the criu-coordinator client would use to connect to a criu-coordinator server. CR_ID is an identifier of the current CRIU instance (e.g., in the case of Kubernetes this could be <pod>-<container>). CR_DEPS contains a colon-separated list of dependencies (i.e., identifiers of other CRIU instances that the current CRIU instance depends on).

[avagin]

@rst0git Do I understand it right that you don't need this change any more?

[rst0git]

@rst0git Do I understand it right that you don't need this change any more?

No, we do need it.

[avagin]

Can we execute criu with all these variables and then action scripts will inherit them?

Yes, ...

Could you answer this question? How do you run criu? Why can't you set this variables for the criu process?

[rst0git]

@avagin I'm sorry, I misunderstood your question. Our goal is enable coordination between multiple criu instances when checkpointing Kubernetes pods. In this case, we use a kubelet API to initiate the container checkpoint via CRI-O and runc. We use the action-env option to setting up the coordination parameters via a configuration file.

[avagin]

Is it a per-container configuration file? Is an action script a shell script? If the answer is yest, can it sources a file with env variables?

[rst0git]

Is it a per-container configuration file?

Yes

Is an action script a shell script? If the answer is yest, can it sources a file with env variables?

Good question. The criu-coordinator action script is a client-server implemented in Rust, but it could load the configuration parameters from a file.

[rst0git]

I'm closing this pull request for now as we have modified criu-coordinator to load the connection parameters from a configuration file.

I've moved the commit adding a test for the --action-script option in #2191.