drbd icon indicating copy to clipboard operation
drbd copied to clipboard

[Docker] Skip actions if the host distro does not match the image distro

Open alexzhc opened this issue 3 years ago • 8 comments
trafficstars

Piraeus kernel-module-injector uses entry.sh from drbd/docker repo.

As discussed with @WanzenBug, a good way to have Piraeus support a Kubernetes cluster with mixed Linux distros, is to have multiple kernel-module-injector initContainers of piraeus-node pods, as demonstrated in drbd-adapter.

Therefore, drbd containers must be able to skip actions (and exit 0) when the host distro does not match the image distro.

Signed-off-by: alexzhc [email protected]

alexzhc avatar Aug 26 '22 10:08 alexzhc

Hi @alexzhc!

Thanks for your contribution to the LINBIT software!

Development for this project happens on mailing lists, rather than on GitHub - this GitHub repository is a read-only mirror that isn't used for accepting contributions. So that your change can become part of our software, please email it to us as a patch.

Here's what to do:

  • Format your contribution
  • Decide where to send your contribution to
  • Set up your system to send your contribution as an email
  • Send your contribution and wait for feedback

How do I format my contribution?

Firstly, all contributions need to be formatted as patches. A patch is a plain text document showing the change you want to make to the code, and documenting why it is a good idea.

You can create patches with git format-patch.

Secondly, patches need 'commit messages', which is the human-friendly documentation explaining what the change is and why it's necessary.

Who do I send my contribution to?

There are two mailing lists:

  • DRBD-dev is "strictly" used for patch coordination.
  • DRBD-user is also fine to send your ideas and initals "RFC" patches. You probably want to start here.

If you're interested in DRBD development, subscribing to these mailing lists is a good idea.

How do I send my contribution?

Use git send-email, which will ensure that your patches are formatted in the standard manner. In order to use git send-email, you'll need to configure git to use your SMTP email server.

For more information about using git send-email, look at the Git documentation or type git help send-email. There are a number of useful guides and tutorials about git send-email that can be found on the internet.

How do I get help if I'm stuck?

Firstly, don't get discouraged, we are here to help! If you are lost in the process, and really tried, you will usually find contact information in header/implementation files, or see who touched the code with git blame. If it was an @linbit.com person, write to them. We are more interested in good patches than strictly following the rules (but you should try first!).

I sent my patch - now what?

You wait.

You can check that your email has been received by checking the mailing list archives for the mailing list you sent your patch to. Messages may not be received instantly, so be patient. Developers are generally very busy people, so it may take a few days, even weeks before your patch is looked at.

Then, you keep waiting. It is fine to kick us again if you did not receive an answer within 2 weeks, but usually we are a lot faster.

Further information

Happy hacking!

This message was posted by a bot - if you have any questions or suggestions, please talk to my owner, @rck

LinbitPRBot avatar Aug 26 '22 10:08 LinbitPRBot

having this MR handled here is fine, reopening

rck avatar Aug 26 '22 11:08 rck

I probably want to have that in a function and maybe do the shell stuff slightly different, but all in all the direction this is going should be fine. I will discuss it with Moritz next week.

rck avatar Aug 26 '22 12:08 rck

one thing we need to keep in mind, I simply don't know that from top of my head: can we assume that $HOSTRELEASE is bind mounted? maybe all the relevant users (e.g., operator) do that, but so far that is only required when we want to fetch packages from LINBIT repos per hash. A very obscure use case, but the only one so far that would need $HOSTRELEASE. no blocker, and something we could handle with some more shell checks, just something to consider

rck avatar Aug 26 '22 12:08 rck

If /etc/host-release is not bind mounted, host_dist will be empty. [ -z $host_dist ] also skips the actions.

yes, a function verify_dist should be better. Let me try emulate the style in other functions.

alexzhc avatar Aug 27 '22 12:08 alexzhc

JFI, this is on hold till the proposed function is provided

rck avatar Sep 06 '22 12:09 rck

@rck, I've pushed the function host_dist_matches_image_dist()

alexzhc avatar Sep 15 '22 09:09 alexzhc

adjusted host_dist_matches_image_dist per above review

alexzhc avatar Sep 21 '22 10:09 alexzhc

hi @alexzhc ,

I did some cleanups (trainling white spaces, spelling), and I think I might want to have that code a bit further down, after the check if the drbd module is already loaded. Guess there is no need to do all the LB_SKIP magic if the module is already there. Would the below version work for you?

diff --git a/docker/entry.sh b/docker/entry.sh
index c78e959ba..e3c478d82 100644
--- a/docker/entry.sh
+++ b/docker/entry.sh
@@ -29,6 +29,23 @@ map_dist() {
        return 0
 }

+host_dist_matches_image_dist() {
+       host_dist="$( map_dist | cut -d'.' -f1 )"
+       if [[ -z "$host_dist" ]] ; then
+               debug "Cannot get host distro!"
+               return 1
+       fi
+
+       image_dist="$( lbdisttool.py -l | cut -d'.' -f1 )"
+       if [[ "$host_dist" == "$image_dist" ]]; then
+               debug "The host distro matches image distro!"
+               return 0
+       else
+               debug "The host distro does not match image distro"
+               return 1
+       fi
+}
+
 print_drbd_version() {
        echo
        echo "DRBD version loaded:"
@@ -234,6 +251,16 @@ if grep -q '^drbd ' /proc/modules; then
        exit 0
 fi

+# LB_SKIP
+# allows skipping if the linux distro of the host does not match that of this image
+# Allow "exit 0", so that when used as an initContainer in Kubernetes,
+# next initContainer with a different distro will be tried
+if [[ $LB_SKIP == yes ]]; then
+       host_dist_matches_image_dist || exit 0
+elif [[ $LB_SKIP == no ]]; then
+       host_dist_matches_image_dist || exit 1
+fi
+
 pkgdir=/tmp/pkg
 kodir=/tmp/ko
 rm -rf "$pkgdir" "$kodir"

rck avatar Sep 26 '22 12:09 rck

hi, @rck. The idea is make it possible to exit 0 when both of the below conditions are met

  • the kernel module is not loaded
  • the image distro does not match the host distro

so that the kubernetes may skip the current initContainer and start the next intiContainer.

If exit 1 under such conditions, the kubernetes will stop the pod at the failed initContainer and never start the next.

alexzhc avatar Oct 09 '22 04:10 alexzhc

@alexzhc I get that, there is no set -e, it would just try to load the deps but not fail. The idea was to not run any code (besides depencency loading) if the DRBD module is already there, not even the dirsto-detection. Anyways, one can argue that LB_SKIP has the semantic to not run any further code if host and container don't match, fair enough. Moved that part to the top again. OK?

diff --git a/docker/entry.sh b/docker/entry.sh
index c78e959ba..c5d022676 100644
--- a/docker/entry.sh
+++ b/docker/entry.sh
@@ -29,6 +29,23 @@ map_dist() {
        return 0
 }

+host_dist_matches_image_dist() {
+       host_dist="$( map_dist | cut -d'.' -f1 )"
+       if [[ -z "$host_dist" ]] ; then
+               debug "Cannot get host distro!"
+               return 1
+       fi
+
+       image_dist="$( lbdisttool.py -l | cut -d'.' -f1 )"
+       if [[ "$host_dist" == "$image_dist" ]]; then
+               debug "The host distro matches image distro!"
+               return 0
+       else
+               debug "The host distro does not match image distro"
+               return 1
+       fi
+}
+
 print_drbd_version() {
        echo
        echo "DRBD version loaded:"
@@ -221,6 +238,16 @@ modprobe_deps() {
 }

 ### main
+# LB_SKIP
+# allows skipping (or failing) if the linux distro of the host does not match that of this image
+# Allow "exit 0", so that when used as an initContainer in Kubernetes,
+# next initContainer with a different distro will be tried
+if [[ $LB_SKIP == yes ]]; then
+       host_dist_matches_image_dist || exit 0
+elif [[ $LB_SKIP == no ]]; then
+       host_dist_matches_image_dist || exit 1
+fi
+
 modprobe_deps
 [[ $LB_HOW == "$HOW_DEPSONLY" ]] && { debug "dependencies loading only, exiting now"; exit 0; }

rck avatar Oct 10 '22 07:10 rck

Thanks, I have update the PR per your revision.

alexzhc avatar Oct 11 '22 07:10 alexzhc

thanks, a 'yes' would have been enough. For DRBD we don't do merges, we always rebase, but anyways, I will queue that in internally, it should merged and pushed soon. after that I will close this ticket

rck avatar Oct 11 '22 07:10 rck

This is now on the drbd-9.1 branch as 973ba0c7083bea597c98460a95cd99962da2545b and also got forward merged to master

rck avatar Oct 11 '22 11:10 rck