Bug 1781664

Hi,

what you are doing is:

parent                 ->     child
referencing variable          defining variable isolated_cores
isolated_cores
that doesn't exist

I.e. you are referencing undefined variable, you need to do it the opposite way:

parent                 ->     child
isolated_cores=1              reference/use variable_cores

It's not only about variables, all inheritance in Tuned works (and worked) this way.

(In reply to Jaroslav Škarvada from comment #3)
> Hi,
> 
> what you are doing is:
> 
> parent                 ->     child
> referencing variable          defining variable isolated_cores
> isolated_cores
> that doesn't exist
> 
> I.e. you are referencing undefined variable, you need to do it the opposite
> way:
> 
> parent                 ->     child
> isolated_cores=1              reference/use variable_cores
> 
> It's not only about variables, all inheritance in Tuned works (and worked)
> this way.

Thanks, Jaroslav!  I can see what you're saying and what you describe works.  I
was thinking about the "include" directive more in the sense of C-like #include
or a shell-like "source".  But looking at:
https://github.com/redhat-performance/tuned/blob/master/doc/manual/modules/performance/con_inheritance-between-tuned-profiles.adoc
basically confirms what you're saying even though a bit more detail about variable
scopes would be appreciated in the docs.  I still believe that a way of defining
the variables I describe would be useful to avoid the need for hard-coding these
into parent configuration files.  Would an RFE for something like [global-variables]
make sense?

Hi Jirko,

before discussing an RFE, let's talk about your specific use case. I think we might be able to find a solution without adding any new features.

My guess is that what you really want to do is create a new profile that will hardcode a value of isolated_cores, so that you can use e.g. the cpu-partitioning profile without having to modify /etc/tuned/cpu-partitioning-variables.conf. You can achieve that by creating a profile and a variables file as follows:

$ cat /etc/tuned/foo/tuned.conf
[main]
include=cpu-partitioning

[variables]
include=/etc/tuned/foo-variables.conf

$ cat /etc/tuned/foo-variables.conf
isolated_cores=1

Then simply applying the 'foo' profile should work.

It works by overriding the 'include' in the '[variables]' section, making sure that the assertions in cpu-partitioning are evaluated only after isolated_cores is defined. The realtime profiles use the same approach.

To make your example from comment#0 work, you can simply define 'isolated_cores' in the parent profile (to an arbitrary value; perhaps an empty string). (I think we could add 'isolated_cores=' to our profiles that use isolated_cores.)

$ cat /etc/tuned/parent/tuned.conf
[main]
summary=Isolated cores parent

[variables]
isolated_cores=
isolated_cores_assert_check = \\${isolated_cores}
# Fail if isolated_cores are not set
assert1=${f:assertion_non_equal:isolated_cores defined:${isolated_cores}:${isolated_cores_assert_check}}


Does this solve your problem? If not, can you talk about your use case?

Hi Ondro, thanks for looking into this!

(In reply to Ondřej Lysoněk from comment #5)
> My guess is that what you really want to do is create a new profile that
> will hardcode a value of isolated_cores, so that you can use e.g. the
> cpu-partitioning profile without having to modify
> /etc/tuned/cpu-partitioning-variables.conf.
Exactly.  A child profile wants to include a parent profile + specify
variables for the parent profile.

> $ cat /etc/tuned/foo/tuned.conf
> [main]
> include=cpu-partitioning
> 
> [variables]
> include=/etc/tuned/foo-variables.conf
> 
> $ cat /etc/tuned/foo-variables.conf
> isolated_cores=1
> 
> Then simply applying the 'foo' profile should work.
It does work and thanks for the tip.  However, consider tuned is running
inside a container and you cannot (or you do not want to) directly modify
/etc/tuned/foo-variables.conf in the container.  Then you are out of luck.

> To make your example from comment#0 work, you can simply define
> 'isolated_cores' in the parent profile (to an arbitrary value; perhaps an
> empty string). (I think we could add 'isolated_cores=' to our profiles that
> use isolated_cores.)
> 
> $ cat /etc/tuned/parent/tuned.conf
> [main]
> summary=Isolated cores parent
> 
> [variables]
> isolated_cores=
> isolated_cores_assert_check = \\${isolated_cores}
> # Fail if isolated_cores are not set
> assert1=${f:assertion_non_equal:isolated_cores
> defined:${isolated_cores}:${isolated_cores_assert_check}}
So this is a lot closer to what I'm looking for and this also (nearly)
works.  Semantically, you declare a global variable in the parent profile.
However, after this declaration the assertion doesn't seem to work any 
more -- the test always passes.

What works for me is this:

[main]
summary=Isolated cores parent

[variables]
isolated_cores=
# Fail if isolated_cores are not set
assert1=${f:assertion_non_equal:isolated_cores defined:${isolated_cores}:}

However, this assertion doesn't work for me when you're using the "include" keyword
in the [variables] section, so this likely needs another BZ for the assertion_non_equal
function or the way variable values are expanded.
 
> Does this solve your problem? If not, can you talk about your use case?
I believe we're getting very close to having a solution, thanks again for looking!

(In reply to jmencak from comment #6)
> > $ cat /etc/tuned/foo/tuned.conf
> > [main]
> > include=cpu-partitioning
> > 
> > [variables]
> > include=/etc/tuned/foo-variables.conf
> > 
> > $ cat /etc/tuned/foo-variables.conf
> > isolated_cores=1
> > 
> > Then simply applying the 'foo' profile should work.
> It does work and thanks for the tip.  However, consider tuned is running
> inside a container and you cannot (or you do not want to) directly modify
> /etc/tuned/foo-variables.conf in the container.  Then you are out of luck.

You would not have to modify foo-variables.conf. You could simply ship the file, along with the profile itself, with 'isolated_cores' preset to '1', couldn't you? What is the difference for you between having 'isolated_cores=1' in foo-variables.conf vs. in the profile itself?

(In reply to Ondřej Lysoněk from comment #7)
> > > Then simply applying the 'foo' profile should work.
> > It does work and thanks for the tip.  However, consider tuned is running
> > inside a container and you cannot (or you do not want to) directly modify
> > /etc/tuned/foo-variables.conf in the container.  Then you are out of luck.
> 
> You would not have to modify foo-variables.conf. You could simply ship the
> file, along with the profile itself, with 'isolated_cores' preset to '1',
> couldn't you? What is the difference for you between having
> 'isolated_cores=1' in foo-variables.conf vs. in the profile itself?

So the difference is you'd need to create that file at build time when you're
creating the image for the container vs. specifying the value for the container
at runtime.  The child "profile itself" where you'd define the variables lives
as a object outside of the container (Kubernetes custom resource) and is being
passed to the container with the variable (e.g. isolated_cores=1) set at runtime.
Users can directly modify that child profile and the associated variables.
I try to keep the API (the Kubernetes custom resource) for specifying the profiles
for the container as simple as possible.  Any changes to the API basically mean
you have to live with it and support it.  At the moment, the API to "pass profiles"
into the container is just the tuned profile itself (tuned.conf).  No scripts,
no configuration files (e.g. with isolated_cores=1) can be passed/written into
the container as files.

If the issues with the assertions I raised in comment6 are addressed and the
existing cpu-partitioning/realtime profiles modified to expose the variables
globally, this solves the problem I'm facing now.

I see. Thanks for the explanation.

Would it work for you if we modified our profiles as follows?
https://github.com/olysonek/tuned/commit/d7bb97ff36f63dce0e96418a2db2a3d08976d188

With that patch in place, you should be able to set isolated_cores directly in the profile (tuned.conf). The assertions should also work now.

[main]
include=cpu-partitioning

[variables]
isolated_cores=3

(In reply to Ondřej Lysoněk from comment #9)
> Would it work for you if we modified our profiles as follows?
> https://github.com/olysonek/tuned/commit/
> d7bb97ff36f63dce0e96418a2db2a3d08976d188
> 
> With that patch in place, you should be able to set isolated_cores directly
> in the profile (tuned.conf). The assertions should also work now.
> 
> [main]
> include=cpu-partitioning
> 
> [variables]
> isolated_cores=3

I see, so to me this is beginning to look like all the variables are actually global
and being propagated to the parent profile, only the expansion behaviour in the parent's
[variables] section is a bit "unexpected".  So the commit you mention solves the issue,
and even the associated "no_balance_cores" variable of the cpu-partitioning profile
can be passed from a "child" to "parent" profile.  However, I'm not convinced this is the
right way of fixing the issue.  Nevertheless, I believe we need a solution...

I believe the variable expansion in tuned needs to be documented or (even better) revised
in the tuned code itself..  Do we need a new BZ for this or can it be tracked by this BZ?

(In reply to Ondřej Lysoněk from comment #9)
> I see. Thanks for the explanation.
> 
> Would it work for you if we modified our profiles as follows?
> https://github.com/olysonek/tuned/commit/
> d7bb97ff36f63dce0e96418a2db2a3d08976d188
> 
> With that patch in place, you should be able to set isolated_cores directly
> in the profile (tuned.conf). The assertions should also work now.
> 
> [main]
> include=cpu-partitioning
> 
> [variables]
> isolated_cores=3

Similar approach will be required for realtime profiles.

Jarda and I agreed that what I proposed is a good enough solution, so I finished the fix. Upstream PR:
https://github.com/redhat-performance/tuned/pull/239

(In reply to jmencak from comment #10)
> So the commit you mention solves
> the issue,
> and even the associated "no_balance_cores" variable of the cpu-partitioning
> profile
> can be passed from a "child" to "parent" profile.

Actually I don't think that no_balance_cores worked properly with the previous version of the patch. The current version should handle it though.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (tuned bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2020:3884