diff mbox series

[v3,3/4] ansible_cfg: add callback plugin cli support

Message ID 20250206-ansible_cfg3-v3-3-4588c8c07e22@samsung.com (mailing list archive)
State New
Headers show
Series ansible.cfg: generate with kconfig | expand

Commit Message

Daniel Gomez Feb. 6, 2025, 10:44 a.m. UTC 
From: Daniel Gomez <da.gomez@samsung.com>

To be able to change the Ansible Configuration File callback plugin
via cli.

Example:

make defconfi-* ANSIBLE_CFG_CALLBACK_PLUGIN="debug"

Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
---
 kconfigs/Kconfig.ansible_cfg | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Luis Chamberlain Feb. 6, 2025, 5:07 p.m. UTC | #1 
On Thu, Feb 06, 2025 at 10:44:20AM +0000, da.gomez@kernel.org wrote:
> From: Daniel Gomez <da.gomez@samsung.com>
> 
> To be able to change the Ansible Configuration File callback plugin
> via cli.
> 
> Example:
> 
> make defconfi-* ANSIBLE_CFG_CALLBACK_PLUGIN="debug"

I meant, I don't want to go and modify all the kdevops-ci repos to
handle this and add ANSIBLE_CFG_CALLBACK_PLUGIN="debug" so the next
best thing to make this happen is to just leverage that the CIs
area are already enabling a kconfig CI specific thing and one of them
is the BOOTLINUX_TREE_SET_BY_CLI. The Only CI that does not use that
is .github/workflows/fstests.yml inside kdevops but since that is
part of kdevops you can easily extend that to use :

diff --git a/.github/workflows/fstests.yml b/.github/workflows/fstests.yml
index 0cff94ab64aa..04fa099b30f2 100644
--- a/.github/workflows/fstests.yml
+++ b/.github/workflows/fstests.yml
@@ -38,7 +38,7 @@ jobs:
         run: |
           KDEVOPS_TREE_REF="${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}"
           SHORT_PREFIX="$(echo ${KDEVOPS_TREE_REF:0:12})"
-          make KDEVOPS_HOSTS_PREFIX="$SHORT_PREFIX" defconfig-xfs_reflink_4k
+          make KDEVOPS_HOSTS_PREFIX="$SHORT_PREFIX" ANSIBLE_CFG_CALLBACK_PLUGIN="debug" defconfig-xfs_reflink_4k
 
       - name: Run kdevops make
         run: |

And so other than the above then I think you need the following to make
all the kdevops-ci branches under https://github.com/linux-kdevops/kdevops-ci
work with this new debug thing out of the box:

diff --git a/kconfigs/Kconfig.ansible_cfg b/kconfigs/Kconfig.ansible_cfg
index bf09c368d85ece4d1272dbe378e1c283c9f57b89..7286b0fe502521835a2dada19b9d59207fc37b94 100644
--- a/kconfigs/Kconfig.ansible_cfg
+++ b/kconfigs/Kconfig.ansible_cfg
@@ -1,7 +1,12 @@
+config ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+	bool
+	default $(shell, scripts/check-cli-set-var.sh ANSIBLE_CFG_CALLBACK_PLUGIN)
+
 menu "Ansible Callback Plugin Configuration"
 choice
 	prompt "Ansible Callback Plugin"
-	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI && !BOOTLINUX_TREE_SET_BY_CLI
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI || BOOTLINUX_TREE_SET_BY_CLI
 
 config ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
 	bool "Ansible Debug Callback Plugin"
@@ -13,13 +18,28 @@ config ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
 	help
 	  Dense: https://docs.ansible.com/ansible/latest/collections/community/general/dense_callback.html
 
+config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+	bool "Custom Ansible Callback Plugin"
+	help
+	  This will let you enter in your own Ansible callback plugin
+
 endchoice
 
+if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+
+config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME
+	string "Custom Ansible Callback Plugin Name"
+	default "debug" if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI && BOOTLINUX_TREE_SET_BY_CLI
+	default $(shell, ./scripts/append-makefile-vars.sh $(ANSIBLE_CFG_CALLBACK_PLUGIN)) if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+	default "dense" if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+
+endif # ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+
 config ANSIBLE_CFG_CALLBACK_PLUGIN_STRING
 	string
 	output yaml
 	default "debug" if ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
 	default "dense" if ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
 
 config ANSIBLE_CFG_CALLBACK_PLUGIN_CHECK_MODE_MARKERS
 	bool "check_mode_markers"
Daniel Gomez Feb. 6, 2025, 5:59 p.m. UTC | #2 
On Thu, Feb 06, 2025 at 09:07:19AM +0100, Luis Chamberlain wrote:
> On Thu, Feb 06, 2025 at 10:44:20AM +0000, da.gomez@kernel.org wrote:
> > From: Daniel Gomez <da.gomez@samsung.com>
> > 
> > To be able to change the Ansible Configuration File callback plugin
> > via cli.
> > 
> > Example:
> > 
> > make defconfi-* ANSIBLE_CFG_CALLBACK_PLUGIN="debug"
> 
> I meant, I don't want to go and modify all the kdevops-ci repos to
> handle this and add ANSIBLE_CFG_CALLBACK_PLUGIN="debug" so the next

This makes sense to avoid...

> best thing to make this happen is to just leverage that the CIs
> area are already enabling a kconfig CI specific thing and one of them
> is the BOOTLINUX_TREE_SET_BY_CLI. The Only CI that does not use that

Oh, I see! Thanks for clarifying.

Relating this 2 unrelated settings because of CI it's easier to forget and/or
for others kdevops users/developers to remember. I think I'd rather keep the
defaults as they are (and drop patch 2) and have my own kdevops fragment to
enable my preferences. On the other hand, I still think is a good idea to enable
dense callback plugin by default just because it's easier to read the output
than the current one. And with Ansible verbose patches one can easily add AV=1
(or 1-6) to increase verbosity when something unexpected happens.

> is .github/workflows/fstests.yml inside kdevops but since that is
> part of kdevops you can easily extend that to use :
> 
> diff --git a/.github/workflows/fstests.yml b/.github/workflows/fstests.yml
> index 0cff94ab64aa..04fa099b30f2 100644
> --- a/.github/workflows/fstests.yml
> +++ b/.github/workflows/fstests.yml
> @@ -38,7 +38,7 @@ jobs:
>          run: |
>            KDEVOPS_TREE_REF="${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }}"
>            SHORT_PREFIX="$(echo ${KDEVOPS_TREE_REF:0:12})"
> -          make KDEVOPS_HOSTS_PREFIX="$SHORT_PREFIX" defconfig-xfs_reflink_4k
> +          make KDEVOPS_HOSTS_PREFIX="$SHORT_PREFIX" ANSIBLE_CFG_CALLBACK_PLUGIN="debug" defconfig-xfs_reflink_4k

This is patch 4/4 with the variable order changed. But that should not matter.

>  
>        - name: Run kdevops make
>          run: |
> 
> And so other than the above then I think you need the following to make
> all the kdevops-ci branches under https://github.com/linux-kdevops/kdevops-ci
> work with this new debug thing out of the box:
> 
> diff --git a/kconfigs/Kconfig.ansible_cfg b/kconfigs/Kconfig.ansible_cfg
> index bf09c368d85ece4d1272dbe378e1c283c9f57b89..7286b0fe502521835a2dada19b9d59207fc37b94 100644
> --- a/kconfigs/Kconfig.ansible_cfg
> +++ b/kconfigs/Kconfig.ansible_cfg
> @@ -1,7 +1,12 @@
> +config ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
> +	bool
> +	default $(shell, scripts/check-cli-set-var.sh ANSIBLE_CFG_CALLBACK_PLUGIN)
> +
>  menu "Ansible Callback Plugin Configuration"
>  choice
>  	prompt "Ansible Callback Plugin"
> -	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
> +	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI && !BOOTLINUX_TREE_SET_BY_CLI

IMHO, although git-blame would work, this is a bit tricky to understand they
why.

> +	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI || BOOTLINUX_TREE_SET_BY_CLI
>  
>  config ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
>  	bool "Ansible Debug Callback Plugin"
> @@ -13,13 +18,28 @@ config ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
>  	help
>  	  Dense: https://docs.ansible.com/ansible/latest/collections/community/general/dense_callback.html
>  
> +config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
> +	bool "Custom Ansible Callback Plugin"
> +	help
> +	  This will let you enter in your own Ansible callback plugin
> +
>  endchoice
>  
> +if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
> +
> +config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME
> +	string "Custom Ansible Callback Plugin Name"
> +	default "debug" if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI && BOOTLINUX_TREE_SET_BY_CLI
> +	default $(shell, ./scripts/append-makefile-vars.sh $(ANSIBLE_CFG_CALLBACK_PLUGIN)) if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
> +	default "dense" if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
> +
> +endif # ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
> +
>  config ANSIBLE_CFG_CALLBACK_PLUGIN_STRING
>  	string
>  	output yaml
>  	default "debug" if ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
>  	default "dense" if ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
> +	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
>  
>  config ANSIBLE_CFG_CALLBACK_PLUGIN_CHECK_MODE_MARKERS
>  	bool "check_mode_markers"

Proposal:
a) Drop patch 2 (the part that changes the plugin but keep the task started
option).
b) I'd do the necessary changes in kdevops-ci repo and branches to use the new
cli ansible cfg variable.

My preferred option would be b) :)

I guess, another option is to detect that kdevops is running in "CI mode" (with
GitHub Actions vars [1]) and do your proposed changes above with that. This
option is good if we want to control kdevops CI behaviour within the code
(and affect all kdevops CI instances). I think I prefer the same kdevops
behaviour regardless of the context and control it through command line and
defconfigs.

What do you think?

[1] https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#default-environment-variables

> 
> -- 
> 2.47.2
>
Luis Chamberlain Feb. 6, 2025, 6:09 p.m. UTC | #3 
On Thu, Feb 6, 2025 at 9:59 AM Daniel Gomez <da.gomez@kernel.org> wrote:
> What do you think?

Now that I managed to explain better what I meant and you get it, you
can decide :)

  Luis
Daniel Gomez Feb. 6, 2025, 10 p.m. UTC | #4 
On Thu, Feb 06, 2025 at 10:09:57AM +0100, Luis Chamberlain wrote:
> On Thu, Feb 6, 2025 at 9:59 AM Daniel Gomez <da.gomez@kernel.org> wrote:
> > What do you think?
> 
> Now that I managed to explain better what I meant and you get it, you
> can decide :)

Sure!

I'll try add this first in kdevops-ci mm branch to validate:

commit 4d15aa19ec0102ae4245901565a8b4037e89c341 (HEAD -> mm)
Author: Daniel Gomez <da.gomez@samsung.com>
Date:   Thu Feb 6 20:12:44 2025 +0000

    kdevops-init.yml: use debug callback plugin

    In preparation to commit [1], configure the Ansible callback plugin via
    cli adn set it debug (instead of the new default dense). This ensures we
    keep the verbosity in CI.

    [1]:
    https://lore.kernel.org/all/20250206-ansible_cfg3-v3-2-4588c8c07e22@samsung.com/

    Signed-off-by: Daniel Gomez <da.gomez@samsung.com>

diff --git a/.github/workflows/kdevops-init.yml b/.github/workflows/kdevops-init.yml
index e1de895..496101b 100644
--- a/.github/workflows/kdevops-init.yml
+++ b/.github/workflows/kdevops-init.yml
@@ -161,7 +161,12 @@ jobs:
           echo "kdevops host prefix: $KDEVOPS_HOSTS_PREFIX"
           echo "kdevops defconfig:   defconfig-${{ env.KDEVOPS_DEFCONFIG }}"

-          KDEVOPS_ARGS="KDEVOPS_HOSTS_PREFIX=$KDEVOPS_HOSTS_PREFIX LINUX_TREE=$LINUX_TREE LINUX_TREE_REF=$LINUX_TREE_REF defconfig-${{ env.KDEVOPS_DEFCONFIG }}"
+          KDEVOPS_ARGS="\
+          KDEVOPS_HOSTS_PREFIX=$KDEVOPS_HOSTS_PREFIX \
+          LINUX_TREE=$LINUX_TREE \
+          LINUX_TREE_REF=$LINUX_TREE_REF \
+          ANSIBLE_CFG_CALLBACK_PLUGIN=\"debug\" \
+          defconfig-${{ env.KDEVOPS_DEFCONFIG }}"
           echo "Going to run:"
           echo "make $KDEVOPS_ARGS"

If this runs successfully, I'll propagate the change in the rest of the branches
in kdevops-ci before merging ansible.cfg v3.

But there's currently a permission error that needs manual fixing. Hopefully my
last patch series fixes it for future runs.

> 
>   Luis
Luis Chamberlain Feb. 6, 2025, 10:32 p.m. UTC | #5 
On Thu, Feb 06, 2025 at 11:00:08PM +0100, Daniel Gomez wrote:
> If this runs successfully, I'll propagate the change in the rest of the branches
> in kdevops-ci before merging ansible.cfg v3.

Great.

> But there's currently a permission error that needs manual fixing. Hopefully my
> last patch series fixes it for future runs.

Thanks I could not figure out why that's an issue or why the issue
was introduced.

  Luis
Daniel Gomez Feb. 7, 2025, 7:19 p.m. UTC | #6 
On Thu, Feb 06, 2025 at 02:32:18PM +0100, Luis Chamberlain wrote:
> On Thu, Feb 06, 2025 at 11:00:08PM +0100, Daniel Gomez wrote:
> > If this runs successfully, I'll propagate the change in the rest of the branches
> > in kdevops-ci before merging ansible.cfg v3.
> 
> Great.
> 
> > But there's currently a permission error that needs manual fixing. Hopefully my
> > last patch series fixes it for future runs.
> 
> Thanks I could not figure out why that's an issue or why the issue
> was introduced.

Ok.

Patch 14ad1104 ("scripts/bringup_guestfs.sh: fix permissions for URI session")
in kdevops-history, fixes permissions for $STORAGEDIR ($POOL_PATH/kdevops/
guestfs) which is what kdevops/libvirt needs. However, when that commit was
introduced, $STORAGEDIR was created simply with 'mkdir -p $STORAGEDIR' (no
sudo). Commit 47d19e2 ("bringup_guestfs.sh: few minor fixes for libvirt URI
user") adds $USE_SUDO to the mkdir command above resulting in $STORAGEDIR
inheriting root permissions. Permissions are further down "fixed" for
$STORAGEDIR by changes introduced in commit 14ad1104 but not for $POOL_PATH. So,
the normal kdevops workflow does not trigger the issue. However, when the user
tries to access/delete/etc $POOL_PATH as in this [1] mm kdevops CI job, the user
will not have the required permissions unless sudo is used.

[1] https://github.com/linux-kdevops/linux-mm-kpd/actions/runs/13187146801/job/36811867463

I would summarize the above by adding the following to the commit message:

Fixes: 47d19e2 ("bringup_guestfs.sh: few minor fixes for libvirt URI user").

Let me know if you think is worth adding the text above (or a summary of the
findings) too.

> 
>   Luis
Luis Chamberlain Feb. 7, 2025, 7:28 p.m. UTC | #7 
On Fri, Feb 07, 2025 at 08:19:02PM +0100, Daniel Gomez wrote:
> On Thu, Feb 06, 2025 at 02:32:18PM +0100, Luis Chamberlain wrote:
> > On Thu, Feb 06, 2025 at 11:00:08PM +0100, Daniel Gomez wrote:
> > > If this runs successfully, I'll propagate the change in the rest of the branches
> > > in kdevops-ci before merging ansible.cfg v3.
> > 
> > Great.
> > 
> > > But there's currently a permission error that needs manual fixing. Hopefully my
> > > last patch series fixes it for future runs.
> > 
> > Thanks I could not figure out why that's an issue or why the issue
> > was introduced.
> 
> Ok.
> 
> Patch 14ad1104 ("scripts/bringup_guestfs.sh: fix permissions for URI session")
> in kdevops-history, fixes permissions for $STORAGEDIR ($POOL_PATH/kdevops/
> guestfs) which is what kdevops/libvirt needs. However, when that commit was
> introduced, $STORAGEDIR was created simply with 'mkdir -p $STORAGEDIR' (no
> sudo). Commit 47d19e2 ("bringup_guestfs.sh: few minor fixes for libvirt URI
> user") adds $USE_SUDO to the mkdir command above resulting in $STORAGEDIR
> inheriting root permissions. Permissions are further down "fixed" for
> $STORAGEDIR by changes introduced in commit 14ad1104 but not for $POOL_PATH. So,
> the normal kdevops workflow does not trigger the issue. However, when the user
> tries to access/delete/etc $POOL_PATH as in this [1] mm kdevops CI job, the user
> will not have the required permissions unless sudo is used.
> 
> [1] https://github.com/linux-kdevops/linux-mm-kpd/actions/runs/13187146801/job/36811867463
> 
> I would summarize the above by adding the following to the commit message:
> 
> Fixes: 47d19e2 ("bringup_guestfs.sh: few minor fixes for libvirt URI user").
> 
> Let me know if you think is worth adding the text above (or a summary of the
> findings) too.

Makes sense! Pushy away!

  Luis
diff mbox series

Patch

diff --git a/kconfigs/Kconfig.ansible_cfg b/kconfigs/Kconfig.ansible_cfg
index bf09c368d85ece4d1272dbe378e1c283c9f57b89..7286b0fe502521835a2dada19b9d59207fc37b94 100644
--- a/kconfigs/Kconfig.ansible_cfg
+++ b/kconfigs/Kconfig.ansible_cfg
@@ -1,7 +1,12 @@ 
+config ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+	bool
+	default $(shell, scripts/check-cli-set-var.sh ANSIBLE_CFG_CALLBACK_PLUGIN)
+
 menu "Ansible Callback Plugin Configuration"
 choice
 	prompt "Ansible Callback Plugin"
-	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
 
 config ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
 	bool "Ansible Debug Callback Plugin"
@@ -13,13 +18,28 @@  config ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
 	help
 	  Dense: https://docs.ansible.com/ansible/latest/collections/community/general/dense_callback.html
 
+config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+	bool "Custom Ansible Callback Plugin"
+	help
+	  This will let you enter in your own Ansible callback plugin
+
 endchoice
 
+if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+
+config ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME
+	string "Custom Ansible Callback Plugin Name"
+	default $(shell, ./scripts/append-makefile-vars.sh $(ANSIBLE_CFG_CALLBACK_PLUGIN)) if ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+	default "dense" if !ANSIBLE_CFG_CALLBACK_PLUGIN_SET_BY_CLI
+
+endif # ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
+
 config ANSIBLE_CFG_CALLBACK_PLUGIN_STRING
 	string
 	output yaml
 	default "debug" if ANSIBLE_CFG_CALLBACK_PLUGIN_DEBUG
 	default "dense" if ANSIBLE_CFG_CALLBACK_PLUGIN_DENSE
+	default ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM_NAME if ANSIBLE_CFG_CALLBACK_PLUGIN_CUSTOM
 
 config ANSIBLE_CFG_CALLBACK_PLUGIN_CHECK_MODE_MARKERS
 	bool "check_mode_markers"