search

ansible-collections / community.kubernetes

Kubernetes Collection for Ansible
https://galaxy.ansible.com/community/kubernetes
GNU General Public License v3.0
265 stars 104 forks source link

Add optional support for helm diff #355

Closed gravesm closed 3 years ago

gravesm commented 3 years ago
SUMMARY

There are some cases where the existing module has difficulty determining if an upgrade would result in changes. This can particularly be a problem when changes are made to a local chart.

This adds optional support for helm diff. If the plugin is present it will be used. Otherwise, the default implementation will be used and a warning will be issued. One caveat: helm diff does not currently support using a repo url, so the default implementation will be used in this case, as well.

Closes #248

ISSUE TYPE
COMPONENT NAME

helm

ADDITIONAL INFORMATION

Several of our existing tests break when helm diff is installed. This is because our local path test charts don't actually contain any resources so nothing changes when we do any upgrade. Changing the appVersion or adding new values won't make a difference. I have added a simple ConfigMap to our test charts so that when an upgrade happens it will actually change something. This is the reason for all the changes to the existing tests.

codecov[bot] commented 3 years ago

Codecov Report

Merging #355 (5a35725) into main (5a5ed79) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #355   +/-   ##
=======================================
  Coverage   36.80%   36.80%           
=======================================
  Files           3        3           
  Lines         758      758           
  Branches      148      148           
=======================================
  Hits          279      279           
  Misses        430      430           
  Partials       49       49

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5a5ed79...5a35725. Read the comment docs.

Akasurde commented 3 years ago 
ERROR: plugins/modules/helm.py:412:14: blacklisted-name: Black listed name "_"
gravesm commented 3 years ago

@Akasurde yes, that one I'm not worried about. It's an easy fix. The bigger problem is I don't know how to address the yamllint errors. Fixing those errors will break the templates for go.

Akasurde commented 3 years ago

Oh yes. I didn't see that. I will check that now.

gravesm commented 3 years ago

@Akasurde I think I sorted out the linting issues. If there's a better way to do this, I'm happy to be educated. I wish we could ignore a directory, but that doesn't seem possible.

Akasurde commented 3 years ago

@gravesm We can disable yamllint with several ways.

  1. Disable directory - https://yamllint.readthedocs.io/en/stable/configuration.html#ignoring-paths
  2. Disable single line - https://yamllint.readthedocs.io/en/stable/disable_with_comments.html#disabling-checks-for-a-specific-line
  3. Disable single file - https://yamllint.readthedocs.io/en/stable/disable_with_comments.html#disabling-checks-for-all-or-part-of-the-file

I am OK with current configuration as well since I don't see chart files changing any time soon.

github-actions[bot] commented 3 years ago

This repository does not accept pull requests, see the README for details.