Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Incorrect merging of cloud-config files with same stages #1341

Closed
Tracked by #1454
venkatnsrinivasan opened this issue Apr 21, 2023 · 3 comments · Fixed by #1343
Closed
Tracked by #1454

🐛 Incorrect merging of cloud-config files with same stages #1341

venkatnsrinivasan opened this issue Apr 21, 2023 · 3 comments · Fixed by #1343
Assignees
Labels
bug Something isn't working prio: high

Comments

@venkatnsrinivasan
Copy link
Collaborator

Kairos version:

NAME="Ubuntu"
VERSION="20.04.6 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.6 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
KAIROS_NAME="kairos-core-ubuntu-20-lts"
KAIROS_VERSION="v2.0.2"
KAIROS_ID="kairos"
KAIROS_ID_LIKE="kairos-core-ubuntu-20-lts"
KAIROS_VERSION_ID="v2.0.2"
KAIROS_PRETTY_NAME="kairos-core-ubuntu-20-lts v2.0.2"
KAIROS_BUG_REPORT_URL="https://github.com/kairos-io/kairos/issues"
KAIROS_HOME_URL="https://github.com/kairos-io/kairos"
KAIROS_IMAGE_REPO="quay.io/kairos/core-ubuntu-20-lts"
KAIROS_IMAGE_LABEL="latest"
KAIROS_GITHUB_REPO="kairos-io/kairos"
KAIROS_VARIANT="core"
KAIROS_FLAVOR="ubuntu-20-lts"

CPU architecture, OS, and Version:
Linux localhost 5.15.0-69-generic #76~20.04.1-Ubuntu SMP Mon Mar 20 15:54:19 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Describe the bug
Two config files with same stages dont get merged correctly.

To Reproduce
1 . Boot the kairos-ubuntu-20-lts-v2.0.2.iso in manual mode
2. Create two files under /oem with the following entries
10_config.yaml

#node-config
install:
  auto: true
  reboot: false
  poweroff: false
  grub_options:
     extra_cmdline: "console=tty0"
options:
  device: /dev/sda
stages:
  initramfs:
    - users:
        kairos:
          groups:
            - sudo
          passwd: kairos

20_config.yaml

#node-config
stages:
  initramfs:
    - users:
        foo:
          groups:
            - sudo
          passwd: bar
  1. Run kairos-agent install
    Expected behavior
    Both the user entries should be present in the target . Instead the merged config file looks like this
    cat /tmp/xxxx2698469135
#cloud-config

install:
    auto: true
    grub_options:
        extra_cmdline: console=tty0
    poweroff: false
    reboot: false
options:
    device: /dev/sda
stages:
    initramfs:
        - users:
            foo:
                groups:
                    - sudo
                passwd: bar

It does not matter if the stages contain the same steps. e.g. one file can contain initramfs->commands and other contains initramfs->users and only one is present in the result. if we change one of the files to "initramfs.after" it merges correctly

#cloud-config

install:
    auto: true
    grub_options:
        extra_cmdline: console=tty0
    poweroff: false
    reboot: false
options:
    device: /dev/sda
stages:
    initramfs:
        - users:
            kairos:
                groups:
                    - sudo
                passwd: kairos
    initramfs.after:
        - users:
            foo:
                groups:
                    - sudo
                passwd: bar
@venkatnsrinivasan venkatnsrinivasan added the bug Something isn't working label Apr 21, 2023
This was referenced Apr 21, 2023
@mudler mudler changed the title Incorrect merging of cloud-config files with same stages 🐛 Incorrect merging of cloud-config files with same stages Apr 21, 2023
@mudler mudler removed their assignment Apr 21, 2023
@mudler
Copy link
Member

mudler commented Apr 21, 2023

this looks bad, however merging was introduced recently - but I guess this is problematic as you didn't had merges before, and now we have unpredictable results.

Is this blocking you ? I've just pushed it to the next release, but if this is impacting you a lot we will need to catch a patch release

@mauromorales
Copy link
Member

mauromorales commented Apr 21, 2023

I can confirm that I can reproduce.

The issue comes from the mergo merge here

return mergo.Merge(c, newConfig, func(c *mergo.Config) { c.Overwrite = true })

Which seems to be known issue :/ darccio/mergo#204

mauromorales added a commit that referenced this issue Apr 23, 2023
fixes #1341

Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
mauromorales added a commit that referenced this issue Apr 24, 2023
fixes #1341

Signed-off-by: Mauro Morales <mauro.morales@spectrocloud.com>
@mauromorales
Copy link
Member

The issue is in since we introduced mergo, so could be around 1.6.1 if I'm not mistaken, or otherwise just the v2 releases. The fix has been merged, and 2.0.3 should be out soon.

@venkatnsrinivasan thanks for bringing this to our attention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working prio: high
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants