nagios-charmers team mailing list archive

[Alex Kavanagh <alex.kavanagh@xxxxxxxxxxxxx>]

Looks pretty good. I wasn't able to build it on my machine (see below), but that's probably because I'm missing something, but it's not clear what.

Please also see inline comments regarding hard-coding the checksums in the charm code.

Re: error:

$ make build
Makefile:4: Warning JUJU_REPOSITORY was not set, defaulting to /home/alex/Projects/Canonical/other/hw-health-charm
Building charm to base directory /home/alex/Projects/Canonical/other/hw-health-charm
build: Destination charm directory: /home/alex/Projects/Canonical/other/hw-health-charm/builds/hw-health
build: Processing layer: layer:options
build: Processing layer: layer:basic
build: Processing layer: layer:status
build: Processing layer: layer:apt
build: Processing layer: layer:nagios
build: Processing layer: layer:snap
build: Processing layer: hw-health (from src)
build: Processing interface: nrpe-external-master
build: Processing interface: juju-info
Traceback (most recent call last):
  File "/snap/charm/176/bin/charm-build", line 9, in <module>
    load_entry_point('charm-tools==2.2.5', 'console_scripts', 'charm-build')()
  File "/snap/charm/176/lib/python3.5/site-packages/charmtools/build/builder.py", line 837, in main
    build()
  File "/snap/charm/176/lib/python3.5/site-packages/charmtools/build/builder.py", line 622, in __call__
    self.generate()
  File "/snap/charm/176/lib/python3.5/site-packages/charmtools/build/builder.py", line 575, in generate
    self.exec_plan(self.plan, self.layers)
  File "/snap/charm/176/lib/python3.5/site-packages/charmtools/build/builder.py", line 548, in exec_plan
    tactic()
  File "/snap/charm/176/lib/python3.5/site-packages/charmtools/build/tactics.py", line 234, in __call__
    self.entity.copy2(target)
  File "/snap/charm/176/usr/lib/python3.5/shutil.py", line 251, in copy2
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/snap/charm/176/usr/lib/python3.5/shutil.py", line 114, in copyfile
    with open(src, 'rb') as fsrc:
FileNotFoundError: [Errno 2] No such file or directory: Path('/home/alex/Projects/Canonical/other/hw-health-charm/src/files/ipmi/check_ipmi_sensor')
Makefile:50: recipe for target 'build' failed
make: *** [build] Error 1


Diff comments:

> diff --git a/.gitignore b/.gitignore
> index 06bf3e8..e808ce9 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -23,3 +23,5 @@ builds
>  
>  # tools bundle (needed for functional testing)
>  tools.zip
> +tools-checksum.zip

It seems a shame to not include these with the charm in a fixtures dir so that, in order to do functional testing, the tester doesn't have to download and create these manually.  This will also make it easier to add this to a CI system.

> +tools-missing.zip
> diff --git a/src/lib/hwhealth/tools.py b/src/lib/hwhealth/tools.py
> index e18b464..fd6692d 100644
> --- a/src/lib/hwhealth/tools.py
> +++ b/src/lib/hwhealth/tools.py
> @@ -270,6 +298,7 @@ class Sas3Ircu(VendorTool):
>      """
>      def __init__(self):
>          super().__init__(shortname='sas3ircu')
> +        self.checksums = ['f150eb37bb332668949a3eccf9636e0e03f874aecd17a39d586082c6be1386bd']

Hard coding the checksums is probably a bad idea.  This means the charm has to change when the associated tools change.  It would be better if a 'build' tool was provided that fetches the required tools, compresses them into the tools.zip and also writes a checksums into a file in the charm directory so that when the charm is built the file is included.  You still get the "checksums with charm" security, but to update the tools, you just have to run the build command again?

>  
>  
>  class Sas2Ircu(VendorTool):


-- 
https://code.launchpad.net/~aluria/hw-health-charm/+git/hw-health-charm/+merge/369278
Your team Nagios Charm developers is subscribed to branch hw-health-charm:master.