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.
--
Mailing list: https://launchpad.net/~nagios-charmers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~nagios-charmers
More help : https://help.launchpad.net/ListHelp
