#1229 Issue `closed`: DRLM ReaR multiple configs support (-C config_name)¶

Labels: enhancement, documentation, fixed / solved / done, external tool

didacog opened issue at 2017-03-09 09:51:¶

Relax-and-Recover (rear) Issue Template¶

Please fill in the following items before submitting a new issue (quick response is not guaranteed with free support):

rear version (/usr/sbin/rear -V): 2.00
Brief description of the issue: Provide support for Multiple backups configuration from DRLM.

Hi @jsmeix,

In order to provide support for Multiple backup methods I'm almost ready to send a PR with other improvements regarding ReaR-DRLM integration, but I want to inform you about this because some code from ReaR main script should be moved to the beginning of the init stage.

lines 417-455 from /usr/sbin/rear should be moved to:

usr/share/rear/init/default/005_set_config_append_files.sh:

if drlm_is_managed ; then
    return 0
fi

# source additional configuration files if specified on the command line:
if test "$CONFIG_APPEND_FILES" ; then
    for config_append_file in $CONFIG_APPEND_FILES ; do
        # If what is specified on the command line starts with '/' an absolute path is meant
        # otherwise what is specified on the command line means a file in CONFIG_DIR.
        # Files in CONFIG_DIR get automatically copied into the recovery system but
        # other files are added to COPY_AS_IS to get them copied into the recovery system:
        case "$config_append_file" in
            (/*)
                config_append_file_path="$config_append_file"
                # If "-C foo" was specified on the command line but 'foo' does not exist
                # try if 'foo.conf' exists and if yes, use that:
                if test -r "$config_append_file_path" ; then
                    COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path" )
                else if test -r "$config_append_file_path.conf" ; then
                         COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path.conf" )
                     else
                         Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                     fi
                fi
                ;;
            (*)
                config_append_file_path="$CONFIG_DIR/$config_append_file"
                ;;
        esac
        # If "-C foo" was specified on the command line but 'foo' does not exist
        # try if 'foo.conf' exists and if yes, use that:
        if test -r "$config_append_file_path" ; then
            LogPrint "Sourcing additional configuration file '$config_append_file_path'"
            Source "$config_append_file_path"
        else if test -r "$config_append_file_path.conf" ; then
                 LogPrint "Sourcing additional configuration file '$config_append_file_path.conf'"
                 Source "$config_append_file_path.conf"
             else
                 Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
             fi
        fi
    done
fi

This way works as expected if not managed by drlm, and in case is managed the configurations are downloaded from DRLM REST api.

I tested it and works as expected, but before the PR I prefer to talk with you about this change.

Regards,

jsmeix commented at 2017-03-09 10:05:¶

@didacog
many thanks for your information in advance!

But I do not understand why the whole support for the
generic '-C' parameter is disabled if drlm_is_managed?

The support for the '-C' parameter is generic functionality
in ReaR which is not only meant for multiple backups
but meant as general useful functionality.

Multiple backups require the '-C' parameter
but the '-C' parameter does not require multiple backups.

Multiple backups is only one particular use case of the
gererically working '-C' parameter.

Wiht what you propose you make the gereric '-C' parameter
a DRLM dependant special case parameter.

This is not the intent of the gereric '-C' parameter.

Simply put:
You cannot cripple a gereric parameter that is described
generically in the rear man page and in 'rear --help'
into a special case thing that depends on DRLM settings.

jsmeix commented at 2017-03-09 10:06:¶

@didacog
when you describe me your reasoning behind why you think
you must disable the '-C' parameter if drlm_is_managed,
I think we can find a solution that works generically
also if drlm_is_managed.

jsmeix commented at 2017-03-09 10:16:¶

@didacog
a very quick offhanded proposal:

Because DRLM_MANAGED=y is set in local.conf
and because local.conf is read before other config files
are read that are specified via the '-C' parameter
I think one can avoid that other config files have any effect
by specifying in the other config files something like

drlm_is_managed && return

My proposal assumes the usr/share/rear/lib/drlm-functions.sh
is read like the other ReaR functions files before rear
reads the config files. If not a generic test for DRLM_MANAGED
must be used.

jsmeix commented at 2017-03-09 10:22:¶

I have tested my above proposal and for me it "just works".

In etc/rear/local.conf I have

DRLM_MANAGED=y

and additionally I have that etc/rear/testy.conf

this_file_name=$( basename ${BASH_SOURCE[0]} )
if drlm_is_managed ; then
    echo DRLM_MANAGED
    return
else
    echo not DRLM_MANAGED
fi
echo reading $this_file_name

and I get

# usr/sbin/rear -d -D -C testy mkrescue
Relax-and-Recover 2.00 / Git
Using log file: /root/rear/var/log/rear/rear-e205.log
Sourcing additional configuration file '/root/rear/etc/rear/testy.conf'
DRLM_MANAGED
ERROR: ReaR only can be run from DRLM Server ('DRLM_MANAGED=y' is set)
Aborting due to an error, check /root/rear/var/log/rear/rear-e205.log for details
Terminated

didacog commented at 2017-03-09 10:37:¶

@jsmeix

I'm not disabling -C confname, just skip loading confname from /etc/rear/* if DRLM_MANAGED=y because those config files will be loaded from DRLM RESTful API.

if not managed ReaR will load confname as usual and returning Error if not found.

didacog commented at 2017-03-09 10:45:¶

@jsmeix see the new code on drlm-functions that will be used in usr/share/rear/init/default/010_set_drlm_env.sh:

function drlm_import_runtime_config() {

    for arg in "${ARGS[@]}" ; do
        key=DRLM_"${arg%%=*}"
        val="${arg#*=}"
        eval $key='$val'
        Log "Setting $key=$val"
    done

    if ! has_binary curl ; then
        Error "Need 'curl' to download DRLM dynamic configuration. Please install curl and try again."
    fi

    if [[ "$DRLM_SERVER" && "$DRLM_REST_OPTS" && "$DRLM_ID" ]]; then
        local DRLM_CFG="/tmp/drlm_config"
        local http_response_code=$(curl $verbose -f -s -S -w '%{http_code}' $DRLM_REST_OPTS -o $DRLM_CFG https://$DRLM_SERVER/clients/$DRLM_ID/config/$CONFIG_APPEND_FILES)
        test "200" = "$http_response_code" || Error "curl failed with HTTP response code '$http_response_code'."
        eval $(cat $DRLM_CFG)
        rm $DRLM_CFG
    else
        Error "ReaR only can be run from DRLM Server ('DRLM_MANAGED=y' is set)"
    fi

}

function drlm_send_log() {

    # send log file in real time to DRLM
    (
    tail -f --lines=5000 --pid=$$ $RUNTIME_LOGFILE | curl $verbose -T- -f -s -S $DRLM_REST_OPTS https://$DRLM_SERVER/client/$DRLM_ID/log/$WORKFLOW/$(date +%
Y%m%d%H%M%S)
    ) &

}

As you can see, the CONFIG_APPEND_FILES will be applied from DRLM:

curl $verbose -f -s -S -w '%{http_code}' $DRLM_REST_OPTS -o $DRLM_CFG https://$DRLM_SERVER/clients/$DRLM_ID/config/$CONFIG_APPEND_FILES

jsmeix commented at 2017-03-09 11:12:¶

I do not like how DRLM inferferes with ReaR's usual behaviour
but when DRLM must inferfere I would prefer to do it directly
where it is currently done i.e. in usr/sbin/rear and not to move
functionality that is done in usr/sbin/rear (i.e. loading config files)
away into a rear script where such functionality is not expected.

For me the following change in usr/sbin/rear
seems to do what you need:

# Finally source additional configuration files if specified on the command line:
if test "$CONFIG_APPEND_FILES" ; then
    # If DRLM_MANAGED=y additional configuration files will be loaded by DRLM:
    if drlm_is_managed ; then
        LogPrint "DRLM_MANAGED: DRLM will load additional configuration files: $CONFIG_APPEND_FILES"
    else
        for config_append_file in $CONFIG_APPEND_FILES ; do
            [...]
        done
    fi
fi

With that I get

# usr/sbin/rear -d -D -C 'foo bar baz' mkrescue
Relax-and-Recover 2.00 / Git
Using log file: /root/rear/var/log/rear/rear-e205.log
DRLM_MANAGED: DRLM will load additional configuration files: foo bar baz
ERROR: ReaR only can be run from DRLM Server ('DRLM_MANAGED=y' is set)
Aborting due to an error, check /root/rear/var/log/rear/rear-e205.log for details
Terminated

If this also works for you I would perefer to have it implemented
this way (i.e. directly in usr/sbin/rear) because this way
it is more obvious where and how DRLM inferferes
with ReaR's usual behaviour (in contrast to when that
is moved away and somewhat 'hidden' in a rear script).

didacog commented at 2017-03-09 11:20:¶

@jsmeix @gdha

No problem for me, I was just following the guidelines from @schlomo when we integrated DRLM first time, that's why the init stage was created for. and assuming that -C loads extra config I assumed that should be moved to that init stage.

jsmeix commented at 2017-03-09 11:20:¶

@gdha
I also assigned you because I would like to get your
opinion about how currently @didacog plans how
DRLM inferferes with ReaR's usual behaviour.

I would prefer when tools that use ReaR do not need
code changes in ReaR but only call usr/sbin/rear with
appropriate parameters and appropriate config files.

From my point of view when tools that use ReaR
need code changes in ReaR, it looks as if something
basic is not well designed in those tools?

jsmeix commented at 2017-03-09 11:22:¶

I don't know about the guidelines from @schlomo
about how tools that use ReaR should do it.
Can I get those guidelines or a URL of them?

jsmeix commented at 2017-03-09 11:23:¶

@schlomo
I also assigned you because I would like to learn
the above mentioned guidelines from you
about how tools that use ReaR should do it.

didacog commented at 2017-03-09 13:19:¶

@jsmeix your proposal here is ok for me, as I said, I just wanted to follow the first integration with rear, but is ok for me the following:

# Finally source additional configuration files if specified on the command line:
if test "$CONFIG_APPEND_FILES" ; then
    # If DRLM_MANAGED=y additional configuration files will be loaded by DRLM:
    if drlm_is_managed ; then
        LogPrint "DRLM_MANAGED: DRLM will load additional configuration files: $CONFIG_APPEND_FILES"
    else
        for config_append_file in $CONFIG_APPEND_FILES ; do
            [...]
        done
    fi
fi

If there is no problem, I will change this way.

Regards,

jsmeix commented at 2017-03-09 14:43:¶

@didacog
at least from me you get hereby a clear OK
to go ahead and change it as written above.

Regardless whether or not that is the final very best way
to implement it, what is most important is that we can
make it working, cf. "Dirty hacks welcome" in
https://github.com/rear/rear/wiki/Coding-Style

jsmeix commented at 2017-03-15 10:18:¶

I think no comments from @gdha or @schlomo
means silent agreement.

didacog commented at 2017-03-15 10:43:¶

I guess so, I have my rear fork with the changes, I'll send the PR after testing them.

gdha commented at 2017-03-15 10:47:¶

ReaR config files have no direct connections with DRLM without that being provided. Therefore, we should be careful with blowing up our main script by trying to source other programs configs. I think that is what @schlomo meant. So, I have not decided yet which way to go. Perhaps @schlomo has no objections, but I cannot speak for him of course
Gratien

Verstuurd vanaf mijn iPhone

Op 15 mrt. 2017 om 11:18 heeft Johannes Meixner notifications@github.com het volgende geschreven:

I think no comments from @gdha or @schlomo
means silent agreement.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

schlomo commented at 2017-03-15 11:47:¶

I went over this entire topic (BTW, first time I consciously notice the extra configs feature so you get my thoughts on that as well) and would like to share my thoughts with you:

About moving the code:

I am in favor of moving stuff from the main script to the init stage. I think that it will help us to make the code more readable and should also bring us closer to writing unit tests for single scriptlets.
I am not entirely happy with the way that DRLM disables ReaR stuff in different places. IMHO we should strive for a better separation of concerns and also accept that a DRLM user could intentionally misconfigure his system. Therefore I would not like to have checks for DRLM mode to prevent loading of extra config or such. If a user calls ReaR manually with strange options under DRLM mode then "shit in - shit out" is fine for me and preferred to adding DRLM checks everywhere.

About CONFIG_APPEND_FILES:

I really would like to have the CONFIG_APPEND_FILES variable as an array to reflect the fact that it contains multiple values.
IMHO CONFIG_APPEND_FILES would be better named ADDITIONAL_CONFIG_FILES or such as there is no appending happening. IMHO renaming this should be painless as the variable is used only internally.
I find the automagic of CONFIG_APPEND_FILES to be dangerous, especially the part where we try adding the .conf suffix. IMHO such an option should take either absolute or relative file names without further guesswork.
I would suggest to add CONFIG_APPEND_FILES (however named) to default.conf and allow users to set this in their own configuration. This goes well together with having it as an array, moving it to the init stage and will make the entire feature much more consistent with ReaR's general behavior.
The help text says that the -C parameter supports a single file while the code supports multiple files (I didn't check the manpage or other docs). If the behavior is changed to use multiple -C foo arguments for multiple configs then this would be also more consistent with Unix command line semantics.

One more thing: I don't understand why the additional config files are not loaded automatically in the rescue system. This feels really off but maybe I don't understand the way how this is supposed to be used. Do we expect the user to remember which -C magic he needs to use with rear recover? From the design perspective everything should work with just a plain rear recover which is why we always generate a specifically tailored rescue image and don't use a generic one.

jsmeix commented at 2017-03-15 12:17:¶

@schlomo
regarding how '-C' is currently meant to be used see
https://github.com/rear/rear/blob/master/doc/user-guide/11-multiple-backups.adoc
and
https://github.com/rear/rear/blob/master/doc/user-guide/12-BLOCKCLONE.adoc

Regarding an automatism for that use case see
https://github.com/rear/rear/issues/1131

ReaR cannot automatically source any additional config files
because the intent behind is that such additional config files
can be used as needed for different mutual exclusive rear runs
for example as described in the above documentation files.

For the whole development story behind have a look at
https://github.com/rear/rear/issues/1088

didacog commented at 2017-03-15 12:39:¶

@jsmeix @schlomo @gdha,

Then I will better move that part of code from the main script to usr/share/rear/init/default/005_set_config_append_files.sh?

I can also set CONFIG_APPEND_FILES (however named) default for the rescue and initialize in default.conf if you want, as I'm working on this.

Regards,

jsmeix commented at 2017-03-15 12:46:¶

Then please do it completely right and move all code
that sources config files out of usr/sbin/rear into
one or more separated 'init' script(s).
Half of the code with same functionality here and
the rest elsewhere makes it nedlessly hard to see
and understand the whole picture which causes
in the future bad adaptions and enhancements
by contributors who fail to see the whole picture.

schlomo commented at 2017-03-15 13:04:¶

@jsmeix I see, thanks for documenting the use case so clearly. I guess that there is a lot of personal taste involved. Personally I would build this with more default behavior and user guidance, e.g. by adding a select block into site.conf that toggles different option sets on recovery or sets them via an extra argument like rear mkbackuponly windows. In total the behavior would be similar, in detail regular rear usage would retain basic or the most common functionality. In any case, those use cases are probably only for the very advanced users and probably won't be used in heavily automated environments.

From a project perspective, I am fine with all that and just ask to keep the code base as clean and consistent as possible (e.g. improve the variable name, use arrays where appropriate and split code into smaller files by topic).

👍 for moving more stuff from the main script to the init stage. The shorter we get the main script the better for everybody.

didacog commented at 2017-03-15 13:38:¶

OK, I will just move back that part of code to init stage, without changing VARIABLE name or changing anything to be default at rescue yet.

Regards,

jsmeix commented at 2017-03-15 13:39:¶

@schlomo
of course I implemented "multiple backups" basically
"according to what I found out what worked easiest for me"
and what was most important for me is that I liked to have it
implemented without too big changes in the ReaR framework, cf.
https://github.com/rear/rear/issues/1088#issuecomment-262981954

Regarding future adaptions and enhancements see
https://github.com/rear/rear/issues/1088#issuecomment-264831535

jsmeix commented at 2017-03-15 13:47:¶

@didacog
when moving sourcing config files into the 'init' stage
be careful how currently it is about

# Now SHARE_DIR CONFIG_DIR VAR_DIR LOG_DIR and KERNEL_VERSION should be set to a fixed value:
readonly SHARE_DIR CONFIG_DIR VAR_DIR LOG_DIR KERNEL_VERSION

Simply put:
When moving any code out of usr/sbin/rear to any later
sourced script, just ensure that nothing could break.

didacog commented at 2017-03-15 13:57:¶

@jsmeix

This should be moved also? I only moved the whole IF statement that was before that line because I did not appreciate any change on those VARIABLES.

jsmeix commented at 2017-03-15 14:10:¶

@didacog
I don't know what exactly you need to do when moving code.
I only liked to point out that when one moves code
one must be extra careful because when code is moved
the execution ordering changes which could result any
unexpected behavioural changes (a.k.a. "regressions").

jsmeix commented at 2017-03-15 14:15:¶

From my point of view your
https://github.com/didacog/rear/commit/95b1fa72f592da596cbce5c6d380598a25eed583
is in the same way wrong as it was wrong before.
At least I will not merge changes that cut code
for one same functionality into pieces and spread it
over different files that are executed at different time.

didacog commented at 2017-03-15 14:19:¶

@jsmeix I'm a bit confused, please can you give me some light on this please. What is wrong? then I could adjust better if needed.

Thanks!

didacog commented at 2017-03-15 14:32:¶

Do you want that merge 005_set_config_append_files.sh and 010_set_drlm_env.sh to same script?

Regards,

didacog commented at 2017-03-15 15:00:¶

@jsmeix This way is better? same functionality in same file. Isn't it?

usr/share/rear/init/default/005_source_additional_config_files.sh


if drlm_is_managed ; then

    PROGS=( "${PROGS[@]}" curl )

    # Needed for curl (HTTPs)
    COPY_AS_IS=( ${COPY_AS_IS[@]} /etc/ssl/certs/* /etc/pki/* )

    LIBS=(
        "${LIBS[@]}"
        /lib*/libnsspem.so*
        /usr/lib*/libnsspem.so*
        /lib*/libfreebl*.so*
        /usr/lib*/libfreebl*.so*
        /lib*/libnss3.so*
        /usr/lib*/libnss3.so*
        /lib*/libnssutil3.so*
        /usr/lib*/libnssutil3.so*
        /lib*/libsoftokn3.so*
        /usr/lib*/libsoftokn3.so*
        /lib*/libsqlite3.so*
        /usr/lib*/libsqlite3.so*
        /lib*/libfreeblpriv3.so*
        /usr/lib*/libfreeblpriv3.so*
        /lib*/libssl.so*
        /usr/lib*/libssl.so*
        /lib*/libnssdbm3.so*
        /usr/lib*/libnssdbm3.so*
    )

    drlm_import_runtime_config
    drlm_send_log

else

    # source additional configuration files if specified on the command line:
    if test "$CONFIG_APPEND_FILES" ; then
        for config_append_file in $CONFIG_APPEND_FILES ; do
            # If what is specified on the command line starts with '/' an absolute path is meant
            # otherwise what is specified on the command line means a file in CONFIG_DIR.
            # Files in CONFIG_DIR get automatically copied into the recovery system but
            # other files are added to COPY_AS_IS to get them copied into the recovery system:
            case "$config_append_file" in
                (/*)
                    config_append_file_path="$config_append_file"
                    # If "-C foo" was specified on the command line but 'foo' does not exist
                    # try if 'foo.conf' exists and if yes, use that:
                    if test -r "$config_append_file_path" ; then
                        COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path" )
                    else if test -r "$config_append_file_path.conf" ; then
                             COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path.conf" )
                         else
                             Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                         fi
                    fi
                    ;;
                (*)
                    config_append_file_path="$CONFIG_DIR/$config_append_file"
                    ;;
            esac
            # If "-C foo" was specified on the command line but 'foo' does not exist
            # try if 'foo.conf' exists and if yes, use that:
            if test -r "$config_append_file_path" ; then
                LogPrint "Sourcing additional configuration file '$config_append_file_path'"
                Source "$config_append_file_path"
            else if test -r "$config_append_file_path.conf" ; then
                     LogPrint "Sourcing additional configuration file '$config_append_file_path.conf'"
                     Source "$config_append_file_path.conf"
                 else
                     Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                 fi
            fi
        done
    fi

fi

# Now SHARE_DIR CONFIG_DIR VAR_DIR LOG_DIR and KERNEL_VERSION should be set to a fixed value:
readonly SHARE_DIR CONFIG_DIR VAR_DIR LOG_DIR KERNEL_VERSION

Maybe this will be better moved to other script after loading config files?

usr/share/rear/init/default/010_send_log_to_drlm.sh

if drlm_is_managed ; then
    drlm_send_log
fi

schlomo commented at 2017-03-15 15:10:¶

@didacog I don't understand why DRLM should disable the additional config files feature (what if a user wants to use DRLM mode through that? Or if a user wants to be able to use ReaR without DRLM through this feature?). For me these are totally unrelated issues. In any case, there are probably tons of other things in ReaR that users could use to shoot themselves in the foot while using DRLM and you don't try to disable all of them.

Can you explain why it is so important for you to disable this feature under DRLM? My suggestion is to keep these two topics unrelated.

BTW, I would also not mind if ReaR would take along curl and its SSL libraries if curl is present. I think that curl is such a useful tool that we should consider that (although wget is not included by default).

The readonly part I would leave in the main script and use it to lock down the important ReaR variables after the init stage. Or put it in some 999_lock_important_variables.sh if it must be in the init stage.

@didacog sorry for being so "difficult" here, I think that all of us ReaR maintainers simply try to use this as a specific example to help us all in finding a common understanding of how ReaR should be written. Thanks a lot for your great patience! Everything you do makes ReaR better!

didacog commented at 2017-03-15 15:12:¶

@schlomo DRLM does not disable this feature, if managed by drlm this config files will be loaded from drlm instead from local fs.

didacog commented at 2017-03-15 15:14:¶

This changes are to support this on DRLM side, not to disable anithing.

see this: https://github.com/didacog/rear/commit/e68d068bcbe05a54a1b9c1696d694354b1569b14#diff-1254e172b4ccbf4754e92b502337cb38

didacog commented at 2017-03-15 15:18:¶

Regarding curl, nothing was changed since DRLM integration with rear (since 1.17) I've just added more libs to the LIBS array to be able to handle SSL certs from rescue image. see: https://github.com/brainupdaters/drlm/issues/42

schlomo commented at 2017-03-15 15:36:¶

@didacog I don't follow. Your proposed code disables loading extra configs in ReaR if DRLM mode is enabled. I would like to see this decoupled. If DRLM also loads extra configs (I presume others than the user has locally) then IMHO this is unrelated.

Or does DRLM actually check how ReaR was called, parse the -C name option itself and then load these configs? That sounds like code duplication to me, why not leave this with ReaR.

didacog commented at 2017-03-15 15:50:¶

@schlomo one of the DRLM features is to provide ReaR configurations from a central management location (DRLM).
In case of

rear -C name

is set, and if ReaR is managed by DRLM, those extra local config files does not be present in the local fs (/etc/rear/name) will be in DRLM. This is why we check if is managed before loading them, because in that case, drlm_import_runtime_config will load them from DRLM server. That's all.
If not checked ReaR will fail because those files are not present, and this is ok, because they are in DRLM.

jsmeix commented at 2017-03-15 16:02:¶

@didacog
many thanks for your explanation in your above
https://github.com/rear/rear/issues/1229#issuecomment-286785456

Now I think it is the first time that I understand the reason behind!
But I will have to think a bit more about it...

For now a very first question:
Why are only additional config files via '-C' affected?
In other words:
Why does DRLM need no DRLM-specific changes
in usr/sbin/rear for all the traditional ReaR config files
like /etc/rear/local.conf , .../site.conf, .../os.conf
and/or when the '-c' option is used?

didacog commented at 2017-03-15 16:09:¶

@jsmeix

in local.conf/site.conf is where you must set DRLM_MANAGED=y.

With just setting that Rear will be able to load the configuration files from DRLM. All rear configurations from DRLM must be loaded after setting just DRLM_MANAGED=y or DRLM_* in local.conf because is the main file to change default.conf settings.

Then all configurations can be managed from DRLM server. This code change is to be able to support 'rear -C config' on DRLM also.

schlomo commented at 2017-03-15 16:14:¶

@didacog let me try in my own words: Do you want to prevent the user from side-loading other local configs under DRLM control?

To help us understand, what would be the problem if your above code would be changed to this:

if drlm_is_managed ; then

    PROGS=( "${PROGS[@]}" curl )

    # Needed for curl (HTTPs)
    COPY_AS_IS=( ${COPY_AS_IS[@]} /etc/ssl/certs/* /etc/pki/* )

    LIBS=(
        "${LIBS[@]}"
        /lib*/libnsspem.so*
        /usr/lib*/libnsspem.so*
        /lib*/libfreebl*.so*
        /usr/lib*/libfreebl*.so*
        /lib*/libnss3.so*
        /usr/lib*/libnss3.so*
        /lib*/libnssutil3.so*
        /usr/lib*/libnssutil3.so*
        /lib*/libsoftokn3.so*
        /usr/lib*/libsoftokn3.so*
        /lib*/libsqlite3.so*
        /usr/lib*/libsqlite3.so*
        /lib*/libfreeblpriv3.so*
        /usr/lib*/libfreeblpriv3.so*
        /lib*/libssl.so*
        /usr/lib*/libssl.so*
        /lib*/libnssdbm3.so*
        /usr/lib*/libnssdbm3.so*
    )

    drlm_import_runtime_config
    drlm_send_log

fi
    # source additional configuration files if specified on the command line:
    if test "$CONFIG_APPEND_FILES" ; then
        for config_append_file in $CONFIG_APPEND_FILES ; do
            # If what is specified on the command line starts with '/' an absolute path is meant
            # otherwise what is specified on the command line means a file in CONFIG_DIR.
            # Files in CONFIG_DIR get automatically copied into the recovery system but
            # other files are added to COPY_AS_IS to get them copied into the recovery system:
            case "$config_append_file" in
                (/*)
                    config_append_file_path="$config_append_file"
                    # If "-C foo" was specified on the command line but 'foo' does not exist
                    # try if 'foo.conf' exists and if yes, use that:
                    if test -r "$config_append_file_path" ; then
                        COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path" )
                    else if test -r "$config_append_file_path.conf" ; then
                             COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path.conf" )
                         else
                             Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                         fi
                    fi
                    ;;
                (*)
                    config_append_file_path="$CONFIG_DIR/$config_append_file"
                    ;;
            esac
            # If "-C foo" was specified on the command line but 'foo' does not exist
            # try if 'foo.conf' exists and if yes, use that:
            if test -r "$config_append_file_path" ; then
                LogPrint "Sourcing additional configuration file '$config_append_file_path'"
                Source "$config_append_file_path"
            else if test -r "$config_append_file_path.conf" ; then
                     LogPrint "Sourcing additional configuration file '$config_append_file_path.conf'"
                     Source "$config_append_file_path.conf"
                 else
                     Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                 fi
            fi
        done
    fi

What would brake in DRLM under this change?

didacog commented at 2017-03-15 16:16:¶

@schlomo
The problem with your suggestion is that these config files will never be present in local fs if DRLM_MANAGED=y and test -r "$config_append_file_path.conf" will fail.

schlomo commented at 2017-03-15 16:26:¶

@didacog That is fine for me. I just want to decouple things which are not related. Is that fine for you, too? Then let's split it up.

didacog commented at 2017-03-15 16:31:¶

@schlomo I do not understood your last comment I guess.

If you are asking if is fine for me that test -r "$config_append_file_path.conf" will fail, no is not fine for me, because then always will fail and rear stop working with error after loading configuration from DRLM fine :-P

schlomo commented at 2017-03-15 16:36:¶

Will this always happen or only if somebody does rear -C name ?

Will it break regular DLRM usage?

Am 15.03.2017 5:31 nachm. schrieb "Didac Oliveira" <notifications@github.com

:

@schlomo https://github.com/schlomo I do not understood your last
comment I guess.

If you are asking if is fine for me that test -r
"$config_append_file_path.conf" will fail, no is not fine for me, because
then always will fail and rear stop working with error. :-P

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rear/rear/issues/1229#issuecomment-286799242, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAGMCICtn2wJvaWwl1Km9hvFWI-VToTaks5rmBJZgaJpZM4MX3EM
.

didacog commented at 2017-03-15 16:38:¶

@schlomo

This will happen if somebody does rear -C name and DRLM_MANAGED=y. And yes will break DRLM usage, because rear will not continue working as expected by DRLM.

didacog commented at 2017-03-15 16:48:¶

@schlomo

If you do not want else statement, maybe this should be ok?

if drlm_is_managed ; then

    PROGS=( "${PROGS[@]}" curl )

    # Needed for curl (HTTPs)
    COPY_AS_IS=( ${COPY_AS_IS[@]} /etc/ssl/certs/* /etc/pki/* )

    LIBS=(
        "${LIBS[@]}"
        /lib*/libnsspem.so*
        /usr/lib*/libnsspem.so*
        /lib*/libfreebl*.so*
        /usr/lib*/libfreebl*.so*
        /lib*/libnss3.so*
        /usr/lib*/libnss3.so*
        /lib*/libnssutil3.so*
        /usr/lib*/libnssutil3.so*
        /lib*/libsoftokn3.so*
        /usr/lib*/libsoftokn3.so*
        /lib*/libsqlite3.so*
        /usr/lib*/libsqlite3.so*
        /lib*/libfreeblpriv3.so*
        /usr/lib*/libfreeblpriv3.so*
        /lib*/libssl.so*
        /usr/lib*/libssl.so*
        /lib*/libnssdbm3.so*
        /usr/lib*/libnssdbm3.so*
    )

    drlm_import_runtime_config
    drlm_send_log
   return 0
fi
    # source additional configuration files if specified on the command line:
    if test "$CONFIG_APPEND_FILES" ; then
        for config_append_file in $CONFIG_APPEND_FILES ; do
            # If what is specified on the command line starts with '/' an absolute path is meant
            # otherwise what is specified on the command line means a file in CONFIG_DIR.
            # Files in CONFIG_DIR get automatically copied into the recovery system but
            # other files are added to COPY_AS_IS to get them copied into the recovery system:
            case "$config_append_file" in
                (/*)
                    config_append_file_path="$config_append_file"
                    # If "-C foo" was specified on the command line but 'foo' does not exist
                    # try if 'foo.conf' exists and if yes, use that:
                    if test -r "$config_append_file_path" ; then
                        COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path" )
                    else if test -r "$config_append_file_path.conf" ; then
                             COPY_AS_IS=( "${COPY_AS_IS[@]}" "$config_append_file_path.conf" )
                         else
                             Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                         fi
                    fi
                    ;;
                (*)
                    config_append_file_path="$CONFIG_DIR/$config_append_file"
                    ;;
            esac
            # If "-C foo" was specified on the command line but 'foo' does not exist
            # try if 'foo.conf' exists and if yes, use that:
            if test -r "$config_append_file_path" ; then
                LogPrint "Sourcing additional configuration file '$config_append_file_path'"
                Source "$config_append_file_path"
            else if test -r "$config_append_file_path.conf" ; then
                     LogPrint "Sourcing additional configuration file '$config_append_file_path.conf'"
                     Source "$config_append_file_path.conf"
                 else
                     Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
                 fi
            fi
        done
    fi

didacog commented at 2017-03-15 16:50:¶

Anyway maybe drlm_send_log should be moved to other script? to keep different things separated

schlomo commented at 2017-03-15 19:52:¶

@didacog this looks much better. I would put all drlm related stuff in a single script in the init stage. And use another script for the config include.

didacog commented at 2017-03-15 21:00:¶

@schlomo

This is how I started, see my initial: https://github.com/rear/rear/issues/1229#issue-212985552
NEW usr/share/rear/init/default/005_set_config_append_files.sh
and
SAME usr/share/rear/init/default/010_set_drlm_env.sh with the drlm_send_log and more LIBS for curl with SSL.

But, as per: https://github.com/rear/rear/issues/1229#issuecomment-286754316 seems better to put in the same script?

jsmeix commented at 2017-03-16 09:12:¶

Regarding "use another script for the config include":

For me it is mandatory (i.e. I will not accept a violation of it)
that one same functionality must be kept together, cf.
https://github.com/rear/rear/issues/1229#issuecomment-286730687

Sourcing the config files was all the time and still is
one same functionality in ReaR.

Therefore sourcing all config files must be kept
in one same script at one same place.

I do not at all care if that one same script which implements
the "sourcing all config files" functionality is usr/sbin/rear
or a separated script in the 'init' stage. If it is a separated script
that script should only implement this one functionality, cf.
https://en.wikipedia.org/wiki/Unix_philosophy
therein in particular "do one thing and do it well".

To make it unmistakably clear what my point is:

I will not accept when the implementation of one single
functionality gets cut into pieces and spread over various places.

I will not do any kind of help or maintenance for scattered code.

On the other hand this means - because ReaR is free software
and I do fully support the ideas behind free software - that
anyone who likes scattered code is free to do what he likes
but then I am also free to not care about such code.

jsmeix commented at 2017-03-16 09:38:¶

@didacog
to better understand your issue:

If in usr/sbin/rear the current

Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."

would be

LogPrint "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."

then - I guess - you would not see an issue
in case of DRLM_MANAGED=y

Reason (as far as I understand it):

I assume that with DRLM_MANAGED=y one can have
arbitrary settings in the local config files but that does
not matter because usr/sbin/rear does not fail when
sourcing the traditional local config files.

Later in some DRLM-specific scripts additional config files
get downloaded from the DRLM server onto the local system
where rear runs and then that downloaded additional config
files get sourced by DRLM-specific scripts.
As far as I understand it this is what the drlm_import_runtime_config
function in lib/drlm-functions.sh does.

This way whatever settings in the initial local config files
could be overwriten when the downloaded additional config
files get later sourced by DRLM-specific scripts.

If my understanding is right, it means:

All what DRLM_MANAGED=y actually means is basically
the same as what '-C' implements:

Additional config files get sourced.

The only difference is that with DRLM_MANAGED=y
the additional config files come from a remote server
while '-C' only works for local files.

If my understanding is still right, it means:

The curent implementations for DRLM_MANAGED=y
and '-C' are two separated implementations for two
times almost the same functionality.

This is bad and I will fix it as soon as I can.

What I propose hereby is:

I will enhance ReaR to support downloading files
from remote via a "DownloadFile" function plus
appropriate config variables to specify the remote stuff
(like the remote server and the download protocol
i.e. the first part of the download URL).

I will enhance ReaR to source config files that need
to be downloaded from a remote server.

This way ReaR will generically support
to be managed from remote by using
config files that exist on a remote server.

When this is done, DRLM_MANAGED=y will be only
one particular use case of that new generic
"Remotely Managed" feature in ReaR.

If my understanding is right and if we agree on a new
"Remotely Managed" feature in ReaR
I will implement it in a way so that it is useful on its own
(which I can test on my systems) and also so that it is
in particular useful for DRLM_MANAGED=y
(which @didacog needs to test on his systems).

This way we can get rid of the current ugly special-case hacks
for DRLM_MANAGED=y in the ReaR code and replace them
with cleanly integrated calls of generic ReaR functionality.

Of course I will happily adapt and enhance and maintain
such generic ReaR functionality in the future.

schlomo commented at 2017-03-16 09:58:¶

@didacog re https://github.com/rear/rear/issues/1229#issuecomment-286877456 What bothered me in your initial suggestion was the first 3 lines where you skip the entire script under DRLM.

To recap what I learned so far:

From DRLM perspective allowing the user to do -C name could be bad but only if the user actually does it. If the user does not use the -C name feature then there is no problem. @didacog is this correct?
@jsmeix wants to keep all related function in one script and not split it. In general I agree, for specific cases we need to discuss. For example, I would prefer to split config loading into 1) load standard configs, 2) load local custom config and 3) load remote config as I perceive those to be sufficiently different topics. I would also write separate unit tests for those, looking at the potential unit tests is always a good indicator for modularizing code. @jsmeix how do you feel about that?
@jsmeix proposes to add a remote configuration feature. I suggested something like that back in 2009 and would strongly suggest to develop this topic from the server side because then it will be much easier to see which client features are required. Specifically I would be careful to create yet another software/config distribution mechanism but rather rely on whatever the admin already employs. I would also like to caution against implementing an RPC-style interface through config loading. As long as ReaR is a command line utility that should remain our primary interface. If a server component wants to interact with it then it needs an RPC component that will then run our ReaR as a command line tool - and potentially add parameters when calling ReaR. Those should however be passed locally and not pulled in from remote to prevent having more than one remote channel.

To get this long discussion of the hook I would like to suggest to accept this PR with the small modification of removing the DRLM check at the beginning.

didacog commented at 2017-03-16 10:14:¶

@jsmeix @schlomo

if this can be changed in the way @jsmeix says:

Error "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."
to
LogPrint "There is '-C $config_append_file' but neither '$config_append_file_path' nor '$config_append_file_path.conf' can be read."

Then, my initial proposal without the check of drlm_managed in
usr/share/rear/init/default/005_set_config_append_files.sh will work with no issues, because rear will not fail if files not found in the local filesystem and all stuff in usr/share/rear/init/default/010_set_drlm_env.sh should work as expected.

This will give us the support of Multiple configs from DRLM RESTful API without the check before trying to load configs -C name.

I all of you are agree, i will adjust those things and send the PR.

We are working hard to provide better ReaR-DRLM integration over the DRLM REST API, more secure and flexible than it was since rear-1.17.

schlomo commented at 2017-03-16 10:20:¶

@didacog sorry, I would prefer to keep the failure if the user does a -C name and name cannot be loaded. ReaR should fail if that what the user said cannot be done.

I don't really understand why this troubles you so much. Why do you think that your users would be using this feature whatsoever? When DRLM calls ReaR you don't use it in any case.

I have the feeling that we are discussing what happens in an extremely specific error situation.

didacog commented at 2017-03-16 10:30:¶

@schlomo adding this support of multiple configs in DRLM will give the users to chose what config to use from DRLM side like in ReaR. Today is not yet ready (in development) in DRLM side, but before we can code this changes in DRLM calling ReaR with -C name we can be able to handle those configs from ReaR with the drlm_import_runtime_config function.

If not, we cannot code this part because ReaR & DRLM cannot handle these extra configs/profiles from DRLM.

You can see here (DRLM develop branch):
https://github.com/brainupdaters/drlm/blob/develop/usr/share/drlm/www/cgi-bin/clients

That DRLM RESTful API is already written to support this, but before calling rear with -C from DRLM first must be integrated in rear.

schlomo commented at 2017-03-16 10:55:¶

OK, I am giving up. You know my opinion - please @jsmeix and @gdha decide how to proceed. I don't want to block anything here.

didacog commented at 2017-03-16 11:10:¶

@schlomo I understand that should fail if configurations not found, This is what I check if drlm_managed before loading or not from local fs.

The problem I see is that you prefer to have 2 separated scripts in 'init' stage and @jsmeix have other opinion. From DRLM side we will send the PR with your recommendations as always, but before that, all of you must be agree how to proceed, I guess

Same script with return 0 at the end of the if drlm_managed statement, or different scripts checking if is drlm_managed at the begining to not finish with error.

From DRLM side we will adjust the code with your decision.

didacog commented at 2017-03-16 11:15:¶

@gdha Do you have an opinion on this? how to proceed better? I'm a bit frustrated at this point. :_(

jsmeix commented at 2017-03-16 11:16:¶

@didacog
don't be frustrated.
I think I can make you happy very soon (i.e. in a few hours).

gdha commented at 2017-03-16 11:50:¶

@didacog I'm fine with the current situation - I think we are on the same page (more or less).

jsmeix commented at 2017-03-16 11:52:¶

With
https://github.com/rear/rear/pull/1247
merged, I hope @didacog can proceed.

Some explanation why I replaced the 'Error' with a 'LogPrint':

On the one hand - from a strict point of view - I regard this as
a step backward that violates "care about possible errors" in
https://github.com/rear/rear/wiki/Coding-Style

On the other hand there is that "Dirty hacks welcome" in
https://github.com/rear/rear/wiki/Coding-Style
which reads

it is more important that the Relax-and-Recover
code works than having nice looking clean code
that sometimes fails

which matches perfectly the current state of this issue here.

I always prefer to move forward little step by little step
(I even prefer to move forward little ugly step by little ugly step)
over big steps that do it right but change a lot at once.

With https://github.com/rear/rear/pull/1247 merged
we can now proceed to get it in the end implemented
in a good and clean way but we are no longer blocked.

In the end what I mean is "Worse is better", see
https://en.wikipedia.org/wiki/Unix_philosophy
i.e. with https://github.com/rear/rear/pull/1247 merged I
"favored simplicity over perfection" to be able to move forward.

didacog commented at 2017-03-16 14:10:¶

@jsmeix

Then with your commit, I will undo some commits on my fork to keep -C name stuff in main script and proceed with the PR.

Hope this night I could have it ready to PR.

Regards,

jsmeix commented at 2017-03-16 14:35:¶

@didacog
yes, please try to keep specific DRLM functionality
separated from generic ReaR functionality.

It is a different and separated issue to move
the whole "sourcing the config files" functionality
out of usr/sbin/rear into one or more new script(s)
in the 'init' stage.
We can do this at any later time as needed and then
with more real experience how the new "multiple backups"
support via DRLM_MANAGED=y works.

didacog commented at 2017-03-16 14:39:¶

Yes, no problem. I just need to undo some changes, or at least create new branch and redo all changes except that movement.

didacog commented at 2017-03-16 15:53:¶

@jsmeix

I have the changes in my fork, I will test all again, because after several changes I want to be sure all is working ok. Then I will send the PR.

jsmeix commented at 2017-03-17 09:34:¶

I submitted https://github.com/rear/rear/issues/1251
to implement a proper 'init' stage in the future.

@didacog
With probability one (https://en.wikipedia.org/wiki/Almost_surely)
you will have to do some further adaptions when I implement
https://github.com/rear/rear/issues/1251
of how specific DRLM functionality in ReaR is implemented
to get it cleaned up as a whole.

didacog commented at 2017-03-17 10:07:¶

@jsmeix

What kind of adaptions? please can you give me more info, because we are working on that part of our code and we should know what is supposed to change.

jsmeix commented at 2017-03-17 10:18:¶

@didacog
I cannot give you info now what you
will have to adapt when I implement
https://github.com/rear/rear/issues/1251
in the future.

For now, just go ahead based on what there is now in ReaR.

In the future, things in ReaR will change
and then you need to adapt (with probability one).

didacog commented at 2017-03-17 10:22:¶

@jsmeix Ok, then will see what will happen :-), but I will appreciate that when you start #1251 we can discuss the changes regarding DRLM integration :-P

didacog commented at 2017-03-22 16:34:¶

Hi all,
I will close this issue because the PR #1252 was merged.

Regards,

[Export of Github issue for rear/rear.]

#1229 Issue closed: DRLM ReaR multiple configs support (-C config_name)¶

didacog opened issue at 2017-03-09 09:51:¶

Relax-and-Recover (rear) Issue Template¶

jsmeix commented at 2017-03-09 10:05:¶

jsmeix commented at 2017-03-09 10:06:¶

jsmeix commented at 2017-03-09 10:16:¶

jsmeix commented at 2017-03-09 10:22:¶

didacog commented at 2017-03-09 10:37:¶

didacog commented at 2017-03-09 10:45:¶

jsmeix commented at 2017-03-09 11:12:¶

didacog commented at 2017-03-09 11:20:¶

jsmeix commented at 2017-03-09 11:20:¶

jsmeix commented at 2017-03-09 11:22:¶

jsmeix commented at 2017-03-09 11:23:¶

didacog commented at 2017-03-09 13:19:¶

jsmeix commented at 2017-03-09 14:43:¶

jsmeix commented at 2017-03-15 10:18:¶

didacog commented at 2017-03-15 10:43:¶

gdha commented at 2017-03-15 10:47:¶

schlomo commented at 2017-03-15 11:47:¶

jsmeix commented at 2017-03-15 12:17:¶

didacog commented at 2017-03-15 12:39:¶

jsmeix commented at 2017-03-15 12:46:¶

schlomo commented at 2017-03-15 13:04:¶

didacog commented at 2017-03-15 13:38:¶

jsmeix commented at 2017-03-15 13:39:¶

jsmeix commented at 2017-03-15 13:47:¶

didacog commented at 2017-03-15 13:57:¶

jsmeix commented at 2017-03-15 14:10:¶

jsmeix commented at 2017-03-15 14:15:¶

didacog commented at 2017-03-15 14:19:¶

didacog commented at 2017-03-15 14:32:¶

didacog commented at 2017-03-15 15:00:¶

schlomo commented at 2017-03-15 15:10:¶

didacog commented at 2017-03-15 15:12:¶

didacog commented at 2017-03-15 15:14:¶

didacog commented at 2017-03-15 15:18:¶

schlomo commented at 2017-03-15 15:36:¶

didacog commented at 2017-03-15 15:50:¶

jsmeix commented at 2017-03-15 16:02:¶

didacog commented at 2017-03-15 16:09:¶

schlomo commented at 2017-03-15 16:14:¶

didacog commented at 2017-03-15 16:16:¶

schlomo commented at 2017-03-15 16:26:¶

didacog commented at 2017-03-15 16:31:¶

schlomo commented at 2017-03-15 16:36:¶

didacog commented at 2017-03-15 16:38:¶

didacog commented at 2017-03-15 16:48:¶

didacog commented at 2017-03-15 16:50:¶

schlomo commented at 2017-03-15 19:52:¶

didacog commented at 2017-03-15 21:00:¶

jsmeix commented at 2017-03-16 09:12:¶

jsmeix commented at 2017-03-16 09:38:¶

schlomo commented at 2017-03-16 09:58:¶

didacog commented at 2017-03-16 10:14:¶

schlomo commented at 2017-03-16 10:20:¶

didacog commented at 2017-03-16 10:30:¶

schlomo commented at 2017-03-16 10:55:¶

didacog commented at 2017-03-16 11:10:¶

didacog commented at 2017-03-16 11:15:¶

jsmeix commented at 2017-03-16 11:16:¶

gdha commented at 2017-03-16 11:50:¶

jsmeix commented at 2017-03-16 11:52:¶

didacog commented at 2017-03-16 14:10:¶

jsmeix commented at 2017-03-16 14:35:¶

didacog commented at 2017-03-16 14:39:¶

didacog commented at 2017-03-16 15:53:¶

jsmeix commented at 2017-03-17 09:34:¶

didacog commented at 2017-03-17 10:07:¶

jsmeix commented at 2017-03-17 10:18:¶

didacog commented at 2017-03-17 10:22:¶

didacog commented at 2017-03-22 16:34:¶

#1229 Issue `closed`: DRLM ReaR multiple configs support (-C config_name)¶