-
Notifications
You must be signed in to change notification settings - Fork 42
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
Could os_patching report on packages requiring a specific version that are not pinned/held/versionlocked? #143
Comments
Hi @ldaneliukas and thanks for opening the issue. You've stumbled into one of the concerns I have with the package resource. You're lucky that the package in question is able to (at least in some way) cope with being patched and then enforced by puppet. The right solution, irrespective of if you're using the os_patching module, is to ensure that any package that you have a specific version requirement for is also "pinned" or "version locked" through the OS tools. I mention in the README that any pinned packages would show up in the
I don't think there is much else we can do from a module perspective to assist with this, but happy to hear any thoughts you might have. Thanks again |
Since you're using a Redhat derivative, you can use the version lock defined type in the yum module |
Thanks for the quick reply @albatrossflavour Pinning packages makes sense and is the first thing that I tried, however, it seems rather overcomplicated. It's perfectly fine, for packages that are installed via hieradata that supplies the information to an underlying profile that utilizes the However, when you have profiles that use the Would it be possible for |
it's an interesting idea. It might be possible to do something, let me have a think about it. I'm not convinced that the module is the right place to handle it, but we might be able to flag potential issues. |
We could put something like this in as an entry in the fact:
Maybe we could match any entries found against the version lock list on the OS and flag any that aren't there in the It'd work for Redhat and Debian. I'll need to talk to @nathangiuliani to see if we could do something similar for Windows. I'm still not convinced we should have this in the module but if we can make it work with relatively little pain, it might be useful. I'm also going to change the title of the issue as it's a little misleading |
Thanks for looking into this. Yes, the snipped you posted is exactly what I was aiming at. I haven't tested the behavior on Windows i.e. how it handles things such as chocolatey versions. I'll do that today and get back to you with any findings that might be of interest. |
It's a bit different with Windows - we are installing OS updates, not updating versions of packages. Generally your approvals are done on an upstream system (e.g. WSUS). We're not dealing with chocolatey here at all. If you're using the Microsoft update catalogue on the internet, you will just get whichever updates you chose based on the options the Windows update UI provides (e.g. whether or not to install recommended updates along with critical, it changes a bit in the newer versions). The closest equivalent I can think of is that you can 'hide' updates, which prevents them from showing as required or being installed. This is the case at least in older versions of Windows (Server 2008 - 2012 R2) - I haven't checked on newer ones. I think our code will install them regardless, as I believe this 'hide' function is part of the UI only, and the APIs will effectively ignore it. It may be worth looking at whether or not we can incorporate this list of hidden updates, but to be honest, I've never seen it used - everyone who wants flexibility just uses WSUS and approves only the updates as per their requirements. |
@ldaneliukas I've got a branch ( feature/new_facter_entries ) which should do what you're after. Can you take a look and let me know what you think. It was a little harder to get it working than I expected as I couldn't use |
I've ran this on Windows and I fully agree with @nathangiuliani that this is not an issue on Windows, since everything is managed via WSUS, hence, I don't think that the module needs to be concerned with it on that front. Thanks for the quick code, I'll replicate the initial test scenario form which this issue has originated and come back with the results. |
@albatrossflavour , can you create a PR for the changes? Unable to comment on the commit, but can't run it due to an error on
kblist .
UpdateI've used commit Unfortunately, there seem to be issues with it:
The update count remains the same with the package as last time and the This can be due to the usage of We could alternatively use |
Give it another try |
Had the chance to test this further and it seems we're almost there 👍 One issue, could we change
$(puppet config print certname --section agent) since the fqdn does not always match certname of the machine which is used as the name for the catalog json. This would make the script more versatile.
Running it yields the following results:
Should we exclude all packages that are os pinned and catalog pinned from |
Great news. I'll fix up the Thoughts? |
Do you mean to block patching completely if there are warnings or just block upgrading the specific packages in the warnings? I'd say the latter would be perfectly fine as it'd solve the issue of the upgrade/downgrade cycle that we're seeing when specific versions are used in the Puppet catalog. If we're talking about blocking the whole upgrade process - I'm not sure how this would solve the original issue as going and pinning all versions found in the warnings can sometimes be unrealistic, e.g. when the versions in the catalog come from different modules/etc. It'd be hard to keep track of these it's even more difficult when Puppet is offered as a service to developers that control their own code. EDIT:
|
I was thinking blocking it completely. I'm thinking of ways of doing it though.
My preference is 3,2,4 (1 isn't an option as it doesn't work everywhere). We could even combine 2, 3 & 4 into one option that says "if we have VSBNL packages should we prevent patching, lock them or continue patching regardless". Irrespective of which way we go, at least for the next few releases, the default will stay as "go ahead and patch" as I do not want to break the workflows for those using the module. |
HAHAHA, crossover posting is always fun! I think I've answered your edit |
Yeah, seems like we are on a similar thought path 😄 I really like the idea of combining 2, 3 & 4 as then the warning section becomes a neat feature that allows admins to control the specific way that the patching is applied, which can mean a different choice being selected for prod/non-prod systems. I'm always for versatility 👍 |
I guess the only thing I didn't think about is that combining the features means I have 3-5x the amount of work to do. I guess it's a good think I'm stuck in a foreign country that has mediocre internet and great craft beer. |
@ldaneliukas are you on the puppet community slack? |
I'm going to proceed with pushing the current code to dev. It seems to be working and has a couple of other features/fixes in it. Then I'll create a new branch to do this work on. |
* #143 try a new way of doing matching for pinned packages * Fixed typos * #147 update the fact with the new data * #147 fix ordering * #147 only create the mismatch array if the file has data in it * #147 cross linux distro compatability * #147 sles compatability * #145 add a list of KB updates to the fact * Fix handling of KBArticleIDs (#146) * Fix handling of KBArticleIDs - Ensures all KBArticleIDs are processed, not just the first one in the array - Ensures the ID's start with "KB", which aids reusing the fact info later on. * fix typo in variable typo in variable (kbslist --> kblist) on line 57 * #143 try a new way of doing matching for pinned packages * Fixed typos * #147 update the fact with the new data * #147 fix ordering * #147 only create the mismatch array if the file has data in it * #147 cross linux distro compatability * #147 sles compatability * #145 add a list of KB updates to the fact * Fix handling of KBArticleIDs (#146) * Fix handling of KBArticleIDs - Ensures all KBArticleIDs are processed, not just the first one in the array - Ensures the ID's start with "KB", which aids reusing the fact info later on. * fix typo in variable typo in variable (kbslist --> kblist) on line 57 * Fix missing slash (#147) * #143 Fix formatting and use puppet_vardir rather than puppet_client_datadir * #143 change to clientcert rather than fqdn for the catalog name
and we're off. |
Yep. I'll message you there.
Wonderful, eager to try this out. |
The idea of locking any package that has a version specified but is not locked is a good one, but I don't think it's going to work. There are too many moving parts between all of the different platforms. I am toying with the idea of coming up with a version locking type/provider, but that's not something I'm going to be doing in the next few weeks. I think the only way forward for what you're looking for is have a flag which will abort the patch run if there are warnings. You can then use the yum/apt argument parameter to exclude the packages. It's not an idea solution but my gut says that this is a problem that will affect a small number of people and would require a substantial investment of time. The right way to fix it is to ensure that any package that needs to be on a specific version is also locked using the OS tools. I will finish the work on having that flag in place. |
My view is that executing os patching is a totally separate thing to version locking of packages, which is really what needs to be done to solve the original issue. Perhaps the best compromise is to make mention of this in this module's documentation so that users are aware they should be looking into version locking if they don't want a patching run through this module to update their packages (which of course, is what would happen by manually patching anyway). FWIW, I've had the same "issue" and version locking is our solution. |
OK, code is updated.
If The default value is false. |
Code complete, closing ticket |
* The declared ISO format does not exist, had one extra `dd` (#141) * Force usage of the 'C' locale (#142) When parsing command output, we should ensure the utility will emit messages in the language we are using for matching patterns. Force the locale to be 'C' by setting the LC_ALL environment variable. * link to the wiki * #128 additional queries as examples * Add missing dependency (#144) The `Exec[os_patching::exec::fact]` requires the `${cache_dir}/reboot_override` file. * Updates to facter and bug fixes (#148) * #143 try a new way of doing matching for pinned packages * Fixed typos * #147 update the fact with the new data * #147 fix ordering * #147 only create the mismatch array if the file has data in it * #147 cross linux distro compatability * #147 sles compatability * #145 add a list of KB updates to the fact * Fix handling of KBArticleIDs (#146) * Fix handling of KBArticleIDs - Ensures all KBArticleIDs are processed, not just the first one in the array - Ensures the ID's start with "KB", which aids reusing the fact info later on. * fix typo in variable typo in variable (kbslist --> kblist) on line 57 * #143 try a new way of doing matching for pinned packages * Fixed typos * #147 update the fact with the new data * #147 fix ordering * #147 only create the mismatch array if the file has data in it * #147 cross linux distro compatability * #147 sles compatability * #145 add a list of KB updates to the fact * Fix handling of KBArticleIDs (#146) * Fix handling of KBArticleIDs - Ensures all KBArticleIDs are processed, not just the first one in the array - Ensures the ID's start with "KB", which aids reusing the fact info later on. * fix typo in variable typo in variable (kbslist --> kblist) on line 57 * Fix missing slash (#147) * #143 Fix formatting and use puppet_vardir rather than puppet_client_datadir * #143 change to clientcert rather than fqdn for the catalog name * rename element to version_specified_but_not_locked_packages * Toggle to allow warnings to block patching #143 (#150) * #143 first stab at a new parameter to control VSBNLP behaviour * Try the new format for the locked files * Abort flag * update fact * #149 allow Debian to run `apt-get autoremove` at reboot (#151) * Add an option to the task to allow the running of `apt-get autoremove` after patching * #149 try both ways of doing the autoremove * #149 fix default * #149 fix default * #149 allow cron job to be removed * #149 revert changes to the task now that we are doing the autoremove via cron * Add an option to the task to allow the running of `apt-get autoremove` after patching * #149 try both ways of doing the autoremove * #149 fix default * #149 fix default * #149 allow cron job to be removed * #149 revert changes to the task now that we are doing the autoremove via cron * Pre-v0.12.0 release
Affected Puppet, Ruby, OS and module versions/distributions
How to reproduce (e.g Puppet code you use)
Add the following code to any Puppet profile/module (use any package you wish):
Run Puppet:
The
/usr/local/bin/os_patching_fact_generation.sh
will update theos_patching
fact:Running the
os_patching
task, will update the version of the package to the latest available version, regardless of the fact, that the version is explicitly set in Puppet itself.Due to this, the next usual
puppet agent
run on the machine, will downgrade the package back to the version set in the catalog:Which in turn will once again update the
os_patching
fact topackage_update_count => 1
and will when ran, will update the same package - a never ending loop of upgrades/downgrades starts.What behaviour did you expect instead
This being a Puppet task, i expected it will honor versions set by Puppet.
Any additional information you'd like to impart
I'm very happy that such a module and initiative exists, however, i'm puzzled as to how would one handle any packages installed via puppet and not set to
installed
orpresent
which is quite dangerous to have in production.Additionaly, what about various modules that might come with certain package installes pinned to specific versions? I assume that these would also result in never ending loops.
The text was updated successfully, but these errors were encountered: