Overview

Request 596323 accepted

autosubmit from concourse

Loading...

Jan Engelhardt's avatar

You should add some >/dev/null, else this might show up on %post..

TARGET SOURCE             FSTYPE OPTIONS
/      /dev/mapper/fjdata btrfs  rw,relatime,attr2,inode64,noquota
TARGET SOURCE             FSTYPE OPTIONS
/var   /dev/mapper/fjdata btrfs  rw,relatime,attr2,inode64,noquota
TARGET SOURCE             FSTYPE OPTIONS
/opt   /dev/mapper/fjdata btrfs  rw,relatime,attr2,inode64,noquota

Richard Brown's avatar

I do not think this will work in the context of SUSE CaaSP or openSUSE Kubic

During a transactional update (the only supported mechanism for updating or installing this package), /var is not mounted

Therefore "! findmount /var" will fail

Therefore it will try and make a /var/lib/cni subvolume with the subvolume of the transactional update

This will then be override by the /var mount on CaaSP and Kubic, but that will not have the directory created

That means in CaaSP or Kubic the entire if findmnt -t btrfs block seems like a no-op that will not accomplish anything you need accomplished

Have you tested this on CaaSP or Kubic? can you please present the logs? Because there is no way I can see this working and would recommend we do NOT merge this.

--

Also, lines 14-15 don't make sense. You are requiring snapper if is_susecaasp, but using snappers mksubvolume on lines 26-27 regardless of whether or not is_susecaasp is true or not.


Richard Brown's avatar

This does not address any of the issues raised in SR# 581729

(Copying here for the sake of redundancy)

I do not think this will work in the context of SUSE CaaSP or openSUSE Kubic

During a transactional update (the only supported mechanism for updating or installing this package), /var is not mounted

Therefore "! findmount /var" will fail

Therefore it will try and make a /var/lib/cni subvolume with the subvolume of the transactional update

This will then be override by the /var mount on CaaSP and Kubic, but that will not have the directory created

That means in CaaSP or Kubic the entire if findmnt -t btrfs block seems like a no-op that will not accomplish anything you need accomplished

Have you tested this on CaaSP or Kubic? can you please present the logs? Because there is no way I can see this working and would recommend we do NOT merge this.

--

Also, lines 14-15 don't make sense. You are requiring snapper if is_susecaasp, but using snappers mksubvolume on lines 26-27 regardless of whether or not is_susecaasp is true or not.


Kiall Mac Innes's avatar

For what it's worth - This is the code Thorsten and yourself provided us to do just this! Do you have an alternative suggestion?


Richard Brown's avatar

we were wrong :) For what it's worth, I certainly didn't think about the implications of running this as part of a transactional update where all of the findmnt's me and thorsten suggested will not work.

But the implications are there - /var will not be mounted during a transactional update

findmnt -s /var (to read the /etc/fstab) might be a workaround

I'd really like to see some tests to make me confident this all works

Also, as this package is creating /var/lib/cni, it really needs to be defined in %files in the specfile - right now this package will be making the folder orphaned, which is really not good.

Request History
Containers Team's avatar

containersteam created request

autosubmit from concourse


Factory Auto's avatar

factory-auto added opensuse-review-team as a reviewer

Please review sources


Factory Auto's avatar

factory-auto added repo-checker as a reviewer

Please review build success


Factory Auto's avatar

factory-auto accepted review

Check script succeeded


Saul Goodman's avatar

licensedigger accepted review

ok


Ismail Dönmez's avatar

namtrac accepted review


Staging Bot's avatar

staging-bot added as a reviewer

Being evaluated by staging project "openSUSE:Factory:Staging:adi:79"


Staging Bot's avatar

staging-bot accepted review

Picked openSUSE:Factory:Staging:adi:79


Repo Checker's avatar

repo-checker accepted review

cycle and install check passed


Staging Bot's avatar

staging-bot accepted review

ready to accept


Staging Bot's avatar

staging-bot approved review

ready to accept


Dominique Leuenberger's avatar

dimstar_suse accepted request

Accept to openSUSE:Factory

openSUSE Build Service is sponsored by