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

ENT-10657: Separated default self upgrade binary version from policy version #2733

Closed
wants to merge 1 commit into from

Conversation

nickanderson
Copy link
Member

@nickanderson nickanderson commented Sep 19, 2023

This improves the behavior for adding masterfiles as a module via repository
URL.

Prior to this change the policy version and default binary version to target for
self upgrade were the same. To align them you either had to explicitly set the
version which becomes a lie about the actual policy version or you would end up
targeting upgrades to a version of binaries that did not exist including the
commit sha of the masterfiles version built from.

With this change, if you simply cfbs add https://github.com/cfengine/masterfiles
or cfbs add https://github.com/cfengine/masterfiles@COMMIT at a commit that was
not a release the policy version attribute in body common control will get a
version with the commit sha and the default binary versions will come from
.CFVERION which should be the current or next release which is the intention of
the default.

TODOs:

  • Is more needed for autogen.sh?
  • Tidy other version alignments (update.cf/current_version).

Ticket: ENT-10657
Changelog: Title

@nickanderson nickanderson added the WIP Work in Progress label Sep 19, 2023
@nickanderson nickanderson marked this pull request as draft September 19, 2023 22:30
@nickanderson nickanderson removed the WIP Work in Progress label Sep 19, 2023
@craigcomstock
Copy link
Contributor

Yeah, I think we need to adjust configure.ac or autogen.sh to use prepare.sh instead of transforming the .in policy files with autotools.

https://github.com/cfengine/masterfiles/blob/master/configure.ac#L241-L248

AC_CONFIG_FILES([Makefile
                controls/update_def.cf
                promises.cf
                standalone_self_upgrade.cf
                tests/Makefile
                tests/acceptance/Makefile
                tests/unit/Makefile
])

So we could remove the three .cf files from AC_CONFIG_FILES and integrate prepare.sh into the build instead of using autotools so that we keep things DRY.

@craigcomstock
Copy link
Contributor

craigcomstock commented Sep 20, 2023

trying to cfbs add from my latest commit on your branch:

cfbs init # but added no masterfiles
cfbs add https://github.com/nickanderson/masterfiles@37260a5319c52eca894c08585f1dc628ac9371e7
cfbs build

out/masterfiles/controls/update_def.cf has

      "current_version" string => "3.23.0";
      "current_release" string => "1";

out/masterfiles/standalone self upgrade has

      "pkg_version" string => ifelse( isvariable( "def.cfengine_software_pkg_version" ), $(def.cfengine_software_pkg_version), "3.23.0");
      "pkg_release" string => ifelse( isvariable( "def.cfengine_software_pkg_release" ), $(def.cfengine_software_pkg_release), "1");

So I think this works. Let's test it together a bit though.

@nickanderson
Copy link
Member Author

I initialized a new project and added the repo at the same commit.

nickanderson@precision-5570:/tmp/cfbs1$ cfbs --non-interactive init; cfbs --non-interactive remove masterfiles; cfbs --non-interactive add https://github.com/nickanderson/masterfiles@37260a5319c52eca894c08585f1dc628ac9371e7
Initialized empty Git repository in /tmp/cfbs1/.git/
Committing using git:

[main (root-commit) 2feabec] Initialized a new CFEngine Build project
 1 file changed, 7 insertions(+)
 create mode 100644 cfbs.json

Initialized an empty project called 'Example project' in 'cfbs.json'
Added module: masterfiles
Committing using git:

[main 454ac9c] Added module 'masterfiles'
 1 file changed, 26 insertions(+), 1 deletion(-)

Removing module 'masterfiles'
Committing using git:

[main 6f682cc] Removed module 'masterfiles'
 1 file changed, 7 insertions(+), 32 deletions(-)
 rewrite cfbs.json (88%)

Found 1 modules in 'https://github.com/nickanderson/masterfiles':
  - masterfiles
Added module: masterfiles
Committing using git:

[main 395088d] Added module 'masterfiles'
 1 file changed, 10 insertions(+), 1 deletion(-)

I built it.

nickanderson@precision-5570:/tmp/cfbs1$ cfbs build

Modules:
001 masterfiles @ 37260a5319c52eca894c08585f1dc628ac9371e7 (Downloaded)

Steps:
001 masterfiles : run './prepare.sh -y'
001 masterfiles : copy './' 'masterfiles/'

Generating tarball...

Build complete, ready to deploy 🐿
 -> Directory: out/masterfiles
 -> Tarball:   out/masterfiles.tgz

To install on this machine: sudo cfbs install
To deploy on remote hub(s): cf-remote deploy
nickanderson@precision-5570:/tmp/cfbs1$ cd out/

Then I looked for "current_version" to see how it was set. Looks like it is set correctly, but it also looks like the update_def.cf.in was delivered in the built policy.

nickanderson@precision-5570:/tmp/cfbs1/out$ grep -R '"current_version"' masterfiles
masterfiles/controls/update_def.cf.in:      "current_version" string => "@DEFAULT_SELF_UPGRADE_BINARY_VERSION@";
masterfiles/controls/update_def.cf:      "current_version" string => "3.23.0";

Similarly I looked for " version =>".

nickanderson@precision-5570:/tmp/cfbs1/out$ grep -R " version =>" masterfiles/
masterfiles/update.cf:      version => "update.cf $(update_def.current_version)";
masterfiles/standalone_self_upgrade.cf.in:      version => "CFEngine Standalone Self Upgrade @VERSION@";
masterfiles/standalone_self_upgrade.cf:      version => "CFEngine Standalone Self Upgrade 3.23.0a.37260a531";
masterfiles/promises.cf:      version => "CFEngine Promises.cf 3.23.0a.37260a531";
masterfiles/promises.cf.in:      version => "CFEngine Promises.cf @VERSION@";

I found that the .in files for standalone_self_upgrade.cf and promises.cf were delivered to the built policy.

Other than that, this seems to do what I expect.

@craigcomstock
Copy link
Contributor

With the last commit, I have fixed this with just a "touch" of anti-DRY (find the .in templates in both prepare.sh and render-templates.sh)

Result is good I think with built policy via cfbs

$ find out/masterfiles/ -name '*.in'
$ 

no .in files. :)

@nickanderson
Copy link
Member Author

Yeah, that's what I see.

nickanderson@precision-5570:/tmp/cfbs1$ cfbs --non-interactive remove masterfiles; cfbs --non-interactive add https://github.com/nickanderson/masterfiles@7c9eb26a20b55af4a8e7ab296baea0957c7427f7
Removing module 'masterfiles'
Committing using git:

[main 4f88824] Removed module 'masterfiles'
 1 file changed, 7 insertions(+), 16 deletions(-)
 rewrite cfbs.json (75%)

Found 1 modules in 'https://github.com/nickanderson/masterfiles':
  - masterfiles
Added module: masterfiles
Committing using git:

[main 91b8f31] Added module 'masterfiles'
 1 file changed, 10 insertions(+), 1 deletion(-)

nickanderson@precision-5570:/tmp/cfbs1$ cfbs build

Modules:
001 masterfiles @ 7c9eb26a20b55af4a8e7ab296baea0957c7427f7 (Downloaded)

Steps:
001 masterfiles : run './prepare.sh -y'
001 masterfiles : copy './' 'masterfiles/'

Generating tarball...

Build complete, ready to deploy 🐿
 -> Directory: out/masterfiles
 -> Tarball:   out/masterfiles.tgz

To install on this machine: sudo cfbs install
To deploy on remote hub(s): cf-remote deploy
nickanderson@precision-5570:/tmp/cfbs1$ grep -R '"current_version"' out/masterfiles
out/masterfiles/controls/update_def.cf:      "current_version" string => "3.23.0";
nickanderson@precision-5570:/tmp/cfbs1$ grep -R " version =>" out/masterfiles/
out/masterfiles/update.cf:      version => "update.cf $(update_def.current_version)";
out/masterfiles/standalone_self_upgrade.cf:      version => "CFEngine Standalone Self Upgrade 3.23.0a.7c9eb26a2";
out/masterfiles/promises.cf:      version => "CFEngine Promises.cf 3.23.0a.7c9eb26a2";

render-templates.sh Outdated Show resolved Hide resolved
This improves the behavior for adding masterfiles as a module via repository
URL.

Prior to this change the policy version and default binary version to target for
self upgrade were the same. To align them you either had to explicitly set the
version which becomes a lie about the actual policy version or you would end up
targeting upgrades to a version of binaries that did not exist including the
commit sha of the masterfiles version built from.

With this change, if you simply cfbs add https://github.com/cfengine/masterfiles
or cfbs add https://github.com/cfengine/masterfiles@COMMIT at a commit that was
not a release the policy version attribute in body common control will get a
version with the commit sha and the default binary versions will come from
.CFVERION which should be the current or next release which is the intention of
the default.

Ticket: ENT-10657
Changelog: Title
@nickanderson nickanderson marked this pull request as ready for review September 20, 2023 15:09
@nickanderson
Copy link
Member Author

@cf-bottom jenkins, please!

@cf-bottom
Copy link

@nickanderson
Copy link
Member Author

@cf-bottom jenkins, please!

@cf-bottom
Copy link

@nickanderson
Copy link
Member Author

After some discussion, we decided to drop this and instead consider how to set the default binary version based on the hubs binary version, as that is really the more ideal default state.

So, hub can render a file that contains it's binary version, clients can download that as part of policy update and the content of that file can be used to set the default target binary version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants