puppetlabs / puppet_operational_dashboards

Apache License 2.0
5 stars 22 forks source link

Decreased minimum stdlib version to 8 #214

Closed the-yorkshire-allen closed 5 months ago

the-yorkshire-allen commented 5 months ago

Due to the error in version 2.0.0 relating to installing external packages due to an expired GPG key, customers are stuck when installing the operation dashboard if they are currently not on stdlib >= 9. Customers with large modules estates have difficulties moving all modules beyond version stdlib 9 due to existing compatibility issues.

CLAassistant commented 5 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Chris Allen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

MartyEwings commented 5 months ago

@the-yorkshire-allen can you elaborate on how this is currently impacting users? The currently supported version of this module is tested only on stdlib 9 and above, im not sure why i would drop it below there and potentially introduce issues

bastelfreak commented 5 months ago

I don't know if this module has a hard dependency to stdlib 9 or newer, but the telegraf module has and we won't downgrade the code in the telegraf module.

I don't understand why users are unable to update their ancient code but want to use the newest version of puppetlabs/puppet_operational_dashboards? Why not use an older version of puppetlabs/puppet_operational_dashboards? Or well, finally work on the module upgrades. It has to happen at some point.

m0dular commented 5 months ago

I'm inclined to close this PR for the reasons bastelfreak mentioned. The latest version of this module and all dependencies are tested with stdlib >= 9.0, and it sounds like that would break Telegraf and possibly other things. Downgrading the stdlib dependency would basically be introducing an older, untested configuration into the latest version of the module.

If the problem preventing a previous version from being used is one of the GPG keys, that could probably be worked around. Maybe with the override for apt that we removed here, or setting some of the gpg-related parameters detailed in the README

m0dular commented 5 months ago

Creating a new environment for the ops dashboard node to run in with stdlib >= 9.0 and all other dependencies satisfied should also work.

the-yorkshire-allen commented 5 months ago

I'm inclined to close this PR for the reasons bastelfreak mentioned.

Agreed.

the-yorkshire-allen commented 5 months ago

I don't understand why users are unable to update their ancient code but want to use the newest version of puppetlabs/puppet_operational_dashboards?

This all came down to an attitude to risk. They have in excess of 30 forge modules in their puppetfile and the stdlib upgrade affected about half that list. The impact to the change was over 10,000 servers and the change management process couldn't align the change window for that many servers. As mentioned, this is primarily due to neglect in not having the modules upgraded in years.

Aaronoftheages commented 5 months ago

I'm going to have to close this, as outlined by @m0dular and @bastelfreak, introducing old versions into the system is going to require development time to introduce other older versions of stdlib to dependant modules, increasing the maintenance burden and cost in a domino effect.

Unfortunately the customer will just have to upgrade to stdlib 9.

bastelfreak commented 5 months ago

This all came down to an attitude to risk.

I think one root cause is the lack of testing infra on their end. With catalog-diff and beaker it's really really use to test such module upgrades and to calculate the impact. And based on that it enables users to upgrade modules more frequently. I implemented that for a few customers and it works really well.

the-yorkshire-allen commented 5 months ago

I think one root cause is the lack of testing infra on their end. With catalog-diff and beaker it's really really use to test such module upgrades and to calculate the impact. And based on that it enables users to upgrade modules more frequently. I implemented that for a few customers and it works really well.

Sadly, this is where process gets in the way of common sense. Due to the modules being required for catalogs in production systems, even if there was no actual change, the code release flagged an impact which meant the servers required a change control window. This is probably why all the modules are so outdated on their system.

It sounds like the best solution here is to get some backwards compatibility in stdlib >= 9 to create a phased upgrade path.

MartyEwings commented 5 months ago

@the-yorkshire-allen i cant speak for the team that develops stdlib, but i have made several contributions, the things that where removed in stdlib9 had been deprecated and had been producing warnings, in some cases up to 6 years.

There is a case of, sometimes the user needs to get out of their own way here, code cant stay backwards compatible for ever, the maintenance costs are outstanding and you end up with a poorer product

m0dular commented 5 months ago

Is creating a new environment for the node running ops dashboards an option? I feel like that is a relatively low effort solution that won't require any code changes.