search

saltstack-formulas / salt-formula

Yes, Salt can Salt itself!
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
197 stars 423 forks source link

feat(package): reorg version-locking salt packages #525

Closed mdschmitt closed 10 months ago

mdschmitt commented 2 years ago

PR progress checklist (to be filled in by reviewers)

What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

Possibly. I'm not 100% sure how pkg.installed's "hold" parameter functions cross-platform. My testing was on an AmazonLinux 2 box, and I don't see anything specific in the integration tests for this particular item..

Related issues and/or pull requests

Describe the changes you're proposing

Add the ability to version-lock the yum packages for Salt, if desired.

Pillar / config required to test the proposed changes

On a RHEL-based system:

---
salt:
  version: 3004
  pin_version: True

Debug log showing how the proposed changes work

Documentation checklist

Testing checklist

Additional context

hatifnatt commented 2 years ago

Version hold is looks very similar with version pinning but it is not exactly the same thing. If I'm not mistaken hold will only hold explicitly installed packages, and pin can pin packages by mask - in current case it's salt* That's the main reason why in my original RP #458 I'm using pinning and not hold. By using pinning you can successfully downgrade all salt packages

allow package downgrade, simple version specification is not enough for downgrade because related packages won't be downgraded, i.e. if you try to downgrade from version 3000 to 2019.2.3 with apt-get install salt-minion=2019.2.* you will face error because apt will try to install version 3000 of salt-common

I'm almost certain that is not possible by simple hold.

mdschmitt commented 2 years ago

@hatifnatt maybe you could explain the difference between "pinning" and "hold"? On the surface, they sound like the same thing... I'm having a hard time finding anything in the official Salt module index specifically about "pinning."

mdschmitt commented 2 years ago

Regarding hold being able to address wildcards like "salt*" I think that it can like so:

[me@myserver pluginconf.d]$ sudo yum versionlock add salt\*
Loaded plugins: extras_suggestions, langpacks, priorities, update-motd, versionlock
Adding versionlock on: 0:salt-master-3004-1.amzn2
Adding versionlock on: 0:salt-3004-1.amzn2
Adding versionlock on: 0:salt-minion-3004-1.amzn2
versionlock added: 3
hatifnatt commented 2 years ago

https://github.com/saltstack-formulas/salt-formula/pull/525#issuecomment-985002785 I don't think I can explain better than I have already did in message above.

Regarding hold being able to address wildcards like "salt*" I think that it can like so:

But can you hold package prior installation? Does hold help you with downgrade? Main reason why I'm using pinning - it helps with downgrade. Also in the formula you only set hold for, let's call it "main" packages i.e. salt-minion or salt-master, salt-common still won't be on hold.

hatifnatt commented 2 years ago

@myii can you please help with failing test? I don't think it's related with PR itself but I'm not familiar with all kitchen stuff.

mdschmitt commented 2 years ago

@hatifnatt I don't know for certain whether or not hold helps with downgrade but I would assume that it handles that case. Maybe someone else knows? I don't have the infra in place to try it out at the moment. :/

I think my question is more along the lines of...should I make this PR a breaking change and replace all the pin_version stuff with the "native" Salt functionality provided by hold? I don't know how to handle the fact that we wouldn't hold salt-common, but I feel like the hard-coded salt* wildcard and /etc/apt/preferences.d/salt file is a little less flexible... What do you think?

Btw, your point about update_holds was totally spot-on. Thanks for that.

mdschmitt commented 2 years ago

Excellent feedback. Give me a little bit to try to clean this up.

mdschmitt commented 2 years ago

I think you're right about the legibility and "less Jinja" suggestion when you say I should do this:

     - hold: {{ salt_settings.get('hold_version', False) }}
     - update_holds: {{ salt_settings.get('update_holds', False ) }}

But unfortunately, if the necessary yum plugin is not installed, having hold and/or update_holds defined in the state resource will outright fail the pkg.installed resource. Example: https://gitlab.com/saltstack-formulas/salt-formula/-/jobs/2272883059#L1879

If you see another cleaner way to do it, I'm all ears.

Should we just install the respective versionlock package regardless of any pillar settings in this formula? That seems a little heavy-handed..

hatifnatt commented 2 years ago

You can have less Jinja something like this:

{% if salt_settings.install_packages and (salt_settings.hold_version or salt_settings.update_holds) %}
include:
  - {{ tplroot }}.hold
{% endif %}

...

salt-master:
    {% if salt_settings.install_packages %}
  pkg.installed:
    ...
    {%- if salt_settings.hold_version or salt_settings.update_holds %}
    - hold: {{ salt_settings.get('hold_version', False) }}
    - update_holds: {{ salt_settings.get('update_holds', False ) }}
    {%- endif %}
    - require:
    {%- if salt_settings.hold_version or salt_settings.update_holds %}
      - sls: {{ tplroot }}.hold
    {%- endif %}

This way versionlock plugin will be installed prior main packages and hold and update_holds values will be added to state only if one of them is true. Note, this code is not verified, so indentation can be incorrect, but I hope you get the idea. Also note - sls: {{ tplroot }}.hold - you can't require sls with relative path.

mdschmitt commented 10 months ago

Nevermind. This is more trouble than it's worth.