-
Notifications
You must be signed in to change notification settings - Fork 185
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
CFE-4408: Fixed package promises with only promisers and no other attributes #5552
Conversation
github CI is green, let's try jenkins @cf-bottom, thanks! |
0ff6857
to
72d6ff6
Compare
Sure, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/10972/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10972/ |
@cf-bottom jenkins on lots of platforms to check this change. |
Alright, I triggered a build: Jenkins: https://ci.cfengine.com/job/pr-pipeline/10988/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-10988/ |
CI was unstable but won't really test this fix anyway. I will manually test packages generated. I don't think there is a reliable way to automate a test since it depends on actual packages. |
72d6ff6
to
07177f7
Compare
This will also fix: https://northerntech.atlassian.net/browse/CFE-4398 |
07177f7
to
804817d
Compare
@basvandervlies maybe take another look at this PR. I have done some testing and think it is good and like you said should resolve two bugs with one stone! 😹 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the log messages!
I do question checking package_module_knowledge.platform_default (an MPFism) vs default:control_common.package_module
(a coreism)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
dc2feb7
to
930fb46
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to my naive eyes.
@cf-bottom jenkins exotics please |
Alright, I triggered a build: (with exotics) Jenkins: https://ci.cfengine.com/job/pr-pipeline/11047/ Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-11047/ |
Only failure in CI is process_test on rhel-7, known issue https://northerntech.atlassian.net/browse/ENT-10994 |
…ributes CFE-4315 introduced a behavior change so that in the case of a simple package promise with only a promiser: packages: "ed"; Things would just work in the case where there was no default package manager defined with package_module_knowledge.platform_default. In the case where package_module_knowledge.platform_default was defined this simple case would cause errors. This change fixes this second case as well as improves logging around the choosing of v1 and v2 package promise implementations. Some of these changes fix CFE-4398 which noted that too many INFO level messages were produced. These messages are moved to DEBUG and VERBOSE level as appropriate. Also added "packages" promises to list of exceptions so that a bare promiser-only package promise does not cause warnings. Ticket: CFE-4408 Changelog: title
930fb46
to
8961a77
Compare
cherry picked to 3.21.x: #5573 |
CFE-4315 introduced a behavior change so that in the case of a simple package promise with only a promiser:
packages:
"ed";
Things would just work in the case where there was no default package manager defined with package_module_knowledge.platform_default.
In the case where package_module_knowledge.platform_default was defined this simple case would cause errors.
This change fixes this second case as well as modifies and adds some logging around the choosing of v1 or v2 package promise implementation.
Ticket: CFE-4408
Changelog: title