FMF metadata support to colin by jscotka · Pull Request #141 · user-cont/colin

jscotka · 2018-05-31T14:11:12Z

FMF Metadata works well

FMF metadata support in colin. There are two different parts

test objects can read FMF metadata from files on demand
scheduler
- dynamic nosetests generator
- FMF rulesets support
- filtering abilities

TODOs:

research how to ideally solve two __init__() or how to check that all variables are set
find way how to name tests
transform all __init__ s to FMF compatible way
find way how transform rulesets to FMF testsets
remove all duplicated stuff from colin what is already implemented by FMF** (next PRs)
- remove duplicated code: Remove duplicated code what is in FMF and colin implementatiopn #155
- add FMF metadata for all testcases: add FMF metadata to all existing testcases #156

Colin output:

$ python -m colin.cli.colin list-checks

maintainer_label
   -> Label 'maintainer' has to be specified.
   -> The name and email of the maintainer (usually the submitter).
   -> https://fedoraproject.org/wiki/Container:Guidelines#LABELS
   -> maintainer, label, required

from_tag_not_latest
   -> In FROM, tag has to be specified and not 'latest'.
   -> Using the 'latest' tag may cause unpredictable builds.It is recommended that a specific tag is used in the FROM.
   -> https://fedoraproject.org/wiki/Container:Guidelines#FROM
   -> from, dockerfile, baseimage, latest, required

maintainer_deprecated
   -> Dockerfile instruction `MAINTAINER` is deprecated.
   -> Replace with label 'maintainer'.
   -> https://docs.docker.com/engine/reference/builder/#maintainer-deprecated
   -> maintainer, dockerfile, deprecated, required

python -m colin.cli.colin check tests/data/Dockerfile

PASS:Label 'maintainer' has to be specified.
PASS:In FROM, tag has to be specified and not 'latest'.
PASS:Dockerfile instruction `MAINTAINER` is deprecated.

PASS:3

FMF output

fmf show --path colin/checks

/best_practices/cmd_or_entrypoint
class: CmdOrEntrypointCheck
description: An ENTRYPOINT allows you to configure a container that will run as an executable. The main purpose of a CMD is to provide defaults for an executing container.
message: Cmd or Entrypoint has to be specified
reference_url: https://fedoraproject.org/wiki/Container:Guidelines#CMD.2FENTRYPOINT_2
tags: cmd and entrypoint
test: best_practices.py

/best_practices/help_file_or_readme
all_must_be_present: False
class: HelpFileOrReadmeCheck
description: Just like traditional packages, containers need some 'man page' information about how they are to be used, configured, and integrated into a larger stack.
files: /help.1 and /README.md
message: The 'helpfile' has to be provided.
reference_url: https://fedoraproject.org/wiki/Container:Guidelines#Help_File
tags: filesystem, helpfile and man
test: best_practices.py

/best_practices/no_root
class: NoRootCheck
description: It can be insecure to run service as root.
message: Service should not run as root by default.
reference_url: ?????
tags: root and user
test: best_practices.py

/dockerfile/from_tag_not_latest
class: FromTagNotLatestCheck
description: Using the 'latest' tag may cause unpredictable builds.It is recommended that a specific tag is used in the FROM.
message: In FROM, tag has to be specified and not 'latest'.
reference_url: https://fedoraproject.org/wiki/Container:Guidelines#FROM
tags: from, dockerfile, baseimage and latest
test: dockerfile.py

/dockerfile/maintainer_deprecated
class: MaintainerDeprecatedCheck
description: Replace with label 'maintainer'.
instruction: MAINTAINER
max_count: 0
message: Dockerfile instruction `MAINTAINER` is deprecated.
reference_url: https://docs.docker.com/engine/reference/builder/#maintainer-deprecated
tags: maintainer, dockerfile and deprecated
test: dockerfile.py

/labels/maintainer_label
class: MaintainerLabelCheck
description: The name and email of the maintainer (usually the submitter).
labels: maintainer
message: Label 'maintainer' has to be specified.
reference_url: https://fedoraproject.org/wiki/Container:Guidelines#LABELS
required: True
tags: maintainer and label
test: labels.py
value_regex: None

when I want to be supercool and have backward compilant output:

use format abilites of fmf:
fmf show --path colin/checks --format "{}\n -> {}\n -> {}\n -> {}\n -> {}\n\n" --value name --value "data['description']" --value "data['message']" --value "data['reference_url']" --value "data['tags']"

/best_practices/cmd_or_entrypoint
    -> An ENTRYPOINT allows you to configure a container that will run as an executable. The main purpose of a CMD is to provide defaults for an executing container.
    -> Cmd or Entrypoint has to be specified
    -> https://fedoraproject.org/wiki/Container:Guidelines#CMD.2FENTRYPOINT_2
    -> ['cmd', 'entrypoint']

/best_practices/help_file_or_readme
    -> Just like traditional packages, containers need some 'man page' information about how they are to be used, configured, and integrated into a larger stack.
    -> The 'helpfile' has to be provided.
    -> https://fedoraproject.org/wiki/Container:Guidelines#Help_File
    -> ['filesystem', 'helpfile', 'man']

/best_practices/no_root
    -> It can be insecure to run service as root.
    -> Service should not run as root by default.
    -> ?????
    -> ['root', 'user']

/dockerfile/from_tag_not_latest
    -> Using the 'latest' tag may cause unpredictable builds.It is recommended that a specific tag is used in the FROM.
    -> In FROM, tag has to be specified and not 'latest'.
    -> https://fedoraproject.org/wiki/Container:Guidelines#FROM
    -> ['from', 'dockerfile', 'baseimage', 'latest']

/dockerfile/maintainer_deprecated
    -> Replace with label 'maintainer'.
    -> Dockerfile instruction `MAINTAINER` is deprecated.
    -> https://docs.docker.com/engine/reference/builder/#maintainer-deprecated
    -> ['maintainer', 'dockerfile', 'deprecated']

/labels/maintainer_label
    -> The name and email of the maintainer (usually the submitter).
    -> Label 'maintainer' has to be specified.
    -> https://fedoraproject.org/wiki/Container:Guidelines#LABELS
    -> ['maintainer', 'label']

Nosetests output - direct

TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py

test (fmf_scheduler.from_tag_not_latest) ... ok
test (fmf_scheduler.maintainer_deprecated) ... ok
test (fmf_scheduler.maintainer_label) ... ok

----------------------------------------------------------------------
Ran 3 tests in 0.003s

OK

using rulesets

RULESETPATH=rulesets/ TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py
test (fmf_scheduler.check@from_tag_not_latest) ... ok
test (fmf_scheduler.check@maintainer_deprecated) ... ok
test (fmf_scheduler.check@maintainer_label) ... ok

advanced filtering

possible to apply them to rulesets or directy to testcases
FILTERS=tags:dockerfile TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py

test (fmf_scheduler.from_tag_not_latest) ... ok
test (fmf_scheduler.maintainer_deprecated) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.001s

OK

$ NAMES="from_tag_not_latest;maintainer_label" TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py

fmf_scheduler.py
test (fmf_scheduler.from_tag_not_latest) ... ok
test (fmf_scheduler.maintainer_label) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.002s

OK

rulesets filters

$ NAMES=default RULESETPATH=rulesets/ TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py

test (fmf_scheduler.check@from_tag_not_latest) ... ok
test (fmf_scheduler.check@maintainer_deprecated) ... ok
test (fmf_scheduler.check@maintainer_label) ... ok

----------------------------------------------------------------------
Ran 3 tests in 0.003s

OK

$ NAMES=fedora RULESETPATH=rulesets/ TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py
fedora ruleset is not given there, so that it runs 0 tests

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK

TomasTomecek

It would be for the best to discuss this in person.

TomasTomecek · 2018-06-01T07:45:04Z

@@ -0,0 +1,19 @@
+description: "asdasdsfdasfsda f"


what format is this? is it yaml?

TomasTomecek · 2018-06-01T07:48:01Z

+    def _get_fmf_metadata(self):
+        output = {}
+        classfile = inspect.getfile(self.__class__)
+        fmf_tree = fmf.Tree(os.path.dirname(classfile))


does this mean that we would have to store the metadata alongside the .py file?

TomasTomecek · 2018-06-01T07:48:56Z

+            setattr(self, k, metadata_dict[k])
+
+    def __init__(self, message=None, description=None, reference_url=None, tags=None):
+        if message or description or reference_url or tags:


better logic could be to load the metadata and just enable overriding them

TomasTomecek

Gotta say I'm pretty lost in the code. I like the change when you removed static data from the code and moved it to fmf metadata. The runner is mystery to me.

TomasTomecek · 2018-06-28T11:09:24Z

-        self.description = description
-        self.reference_url = reference_url
-        self.tags = tags
+    def _fmf_meta_init(self, args, kwargs):


shouldn't this be *args, **kwargs?

could be, but I forwarded these list and dict directly from __init__, so that no need to dereferenced them.

TomasTomecek · 2018-06-28T11:16:11Z

@@ -0,0 +1,115 @@
+import os


What does this script is suppose to be doing?

it does this scheduler, it dynamically generates nosetest testclasses and cases from metadata

I still don't understand how should I use this file:

$ ./fmf_scheduler.py zsh: permission denied: ./fmf_scheduler.py $ python3 ./fmf_scheduler.py /home/tt/.local/lib/python3.6/site-packages/requests/__init__.py:80: RequestsDependencyWarning: urllib3 (1.23) or chardet (3.0.4) doesn't match a supported version! RequestsDependencyWarning) Traceback (most recent call last): File "./fmf_scheduler.py", line 16, in <module> TARGET = Target(os.environ.get("TARGET"), LOG_LEVEL) File "/home/tt/g/user-cont/colin/colin/core/target.py", line 78, in __init__ self.instance = Target._get_target_instance(target, logging_level=logging_level) File "/home/tt/g/user-cont/colin/colin/core/target.py", line 100, in _get_target_instance if os.path.isfile(target): File "/usr/lib64/python3.6/genericpath.py", line 30, in isfile st = os.stat(path) TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType $ python3 ./fmf_scheduler.py registry.access.redhat.com/rhscl/mariadb-101-rhel7:latest /home/tt/.local/lib/python3.6/site-packages/requests/__init__.py:80: RequestsDependencyWarning: urllib3 (1.23) or chardet (3.0.4) doesn't match a supported version! RequestsDependencyWarning) Traceback (most recent call last): File "./fmf_scheduler.py", line 16, in <module> TARGET = Target(os.environ.get("TARGET"), LOG_LEVEL) File "/home/tt/g/user-cont/colin/colin/core/target.py", line 78, in __init__ self.instance = Target._get_target_instance(target, logging_level=logging_level) File "/home/tt/g/user-cont/colin/colin/core/target.py", line 100, in _get_target_instance if os.path.isfile(target): File "/usr/lib64/python3.6/genericpath.py", line 30, in isfile st = os.stat(path) TypeError: stat: path should be string, bytes, os.PathLike or integer, not NoneType

@TomasTomecek I've wrote it to PR description. usage could be via nosetests like:

TARGET=tests/data/Dockerfile nosetests -d -v fmf_scheduler.py

and I've fixed it, and fmf_scheduler.py can be used as scheduler too and fixed there issue when envvar TARGET is not given

TomasTomecek · 2018-06-28T11:21:12Z

+
+
+    def __init__(self, *args, **kwargs):
+        self._fmf_meta_init(args, kwargs)


I don't like that fmf is the only way to configure colin.

It is not just one way. It allows also pass params via normal init
I'll rename it, to another name, like generic_init or something similar

TomasTomecek · 2018-06-28T11:22:08Z

+    description = ""
+    reference_url = ""
+    tags = []
+    base_init_list = ["message", "description", "reference_url", "tags"]


so why not just do __init__(self, message, description, reference_url, tags)?

because this allows me, pass init parameters and assign them as instance variables from fmf data.

TomasTomecek · 2018-06-28T11:25:06Z

+
+    def references(self, patterntree, whole=False):
+        """
+        resolve references in names like /a/b/c/d@.x.y or /a/b/c/@y


what are these references?

these references implements something similar, what are in colin rulesets.

TomasTomecek · 2018-07-02T14:04:43Z

+
+"""
+Module handling FMF stored metadata for classes
+"""


module docstrings are meant to be before imports

TomasTomecek · 2018-07-02T14:07:44Z

@@ -0,0 +1 @@
+1


is this file needed?

yes, this is created by fmf init. This is new feature of fmf.

TomasTomecek · 2018-07-02T15:12:22Z

I like the changes to the test metadata: it's great they are now all static and not embedded in the code.

It seems that the ruleset interface is still functional: I was able to pass our own ruleset just fine.

move fmf metadata loader outsite, to be able to use it more generic add more flexible functions to deal with default values and init values from FMF fist step with meta classes for testing via nosetest changed all core.checks to support fmf input format added fmf file for best practices and added init to all files fix defaut values, remove them from init_list cleanup of fmf init method first working version with fmf metadata for few cases added relevancy filtering to fmf scheduler add fmf rulesets via referencing shorter output when using nose added docs strings and few fixes based on reviews

cleanup, use just one fmf import improve scheduler, allow schedule tests directly via this file (nosetest framework import inside) I hope that this is good enough format for codacy, grrr another codacy fixes

TomasTomecek · 2018-07-04T14:08:33Z

Regarding the test failure, it's failing on maintainer_label check. The check should pass but instead it fails. Since this code changes that check, it's clear to me there is an issue with the new code.

After ~15 minutes of debugging, the problem is that FMF processes

value_regex: None

as string "None", not type None

Interesting that

required: True

is processed correctly.

All attributes of the check:

{'description': 'The name and email of the maintainer (usually the submitter).',
 'labels': ['maintainer'],
 'message': "Label 'maintainer' has to be specified.",
 'reference_url': 'https://fedoraproject.org/wiki/Container:Guidelines#LABELS',
 'required': True,
 'tags': ['maintainer', 'label', 'required'],
 'value_regex': 'None'}

…uleset or select tests based on names or advanced filters

…ible with colin

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

easier to trace issues Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

TomasTomecek · 2018-07-16T13:55:24Z

@@ -0,0 +1,188 @@
+#!/usr/bin/python3


Since this script actually proposes a new runtime, I would personally prefer to have it proposed in a different PR. I actually dislike that it's not integrated into colin's CLI interface.

yep, this is my suggested solution, split this PR, to 2 or 3 PRs, and last of them will be this nosetests generator.

But on other hand, without this, it is harder to show benefits eg:

write tests just in metadata without empty python classes

using nosetests scheduled tests

filtering and selecting or test items

using FMF rulesets

And this PR will just split metadata outside python classes to FMF files.
But I agree and with this knowledge, that these PRs are closely connected I can split it.

TomasTomecek · 2018-07-16T13:58:21Z

-                      reference_url="https://fedoraproject.org/wiki/Container:Guidelines#FROM",
-                      tags=["from", "dockerfile", "baseimage", "latest"])
+class FromTagNotLatestCheck(FMFAbstractCheck, DockerfileAbstractCheck):
+    name, metadata = FMFAbstractCheck.get_metadata("from_tag_not_latest")


I wonder whether there is a nicer way of doing this. Calling functions on class' attributes outside of methods seems sketchy.

Yep, I had also troubles with this, but actually there is probably not better way how to do it without bigger changes in colin. Because current colin implementation does static inspection of classes, to find checks based on class attribute name. If you have some idea I'll be happy to change it to better way

jscotka · 2018-07-17T08:57:07Z

This PR can be closed, PR splitted to more PRs:
#159
#160
#161

jscotka force-pushed the fmf_metadata branch 2 times, most recently from a5c8249 to f8c4ef7 Compare May 31, 2018 14:27

user-cont deleted a comment May 31, 2018

TomasTomecek requested changes Jun 1, 2018

View reviewed changes

jscotka force-pushed the fmf_metadata branch from e0f36d7 to f8afd61 Compare June 1, 2018 16:18

user-cont deleted a comment Jun 1, 2018

user-cont deleted a comment from TomasTomecek Jun 1, 2018

user-cont deleted a comment Jun 1, 2018

jscotka force-pushed the fmf_metadata branch 2 times, most recently from af8f0e9 to caf053c Compare June 4, 2018 08:46

user-cont deleted a comment Jun 4, 2018

user-cont deleted a comment Jun 26, 2018

TomasTomecek reviewed Jun 28, 2018

View reviewed changes

TomasTomecek reviewed Jul 2, 2018

View reviewed changes

jscotkahat added 3 commits July 4, 2018 13:06

try to use another, more explicit way

aa8c891

cleanup, use just one fmf import improve scheduler, allow schedule tests directly via this file (nosetest framework import inside) I hope that this is good enough format for codacy, grrr another codacy fixes

improve rulesets handling

a5f4dee

jscotkahat added 3 commits July 9, 2018 08:13

changed to Null from None, should be tranformed to pyton None

c363e84

reverted typo back

aba58db

add filter possibilities via envvar values, possible to select some r…

f6728d2

…uleset or select tests based on names or advanced filters

This was referenced Jul 9, 2018

Remove duplicated code what is in FMF and colin implementatiopn #155

Open

add FMF metadata to all existing testcases #156

Closed

jscotkahat and others added 9 commits July 9, 2018 12:30

missing important part, what raises exception in case, it fails

0e58bcf

consolidate logging

9b6cbc1

add support for pure fmf classes, without any python file, not compat…

e1dcf4c

…ible with colin

move readinf CHECK envvar to get_check_path, to be compatible with colin

68a98f6

fix instruction checks when *_count is 0

71f2043

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

enable all tests by default

962b845

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

tests: more verbose logs

3229fe5

easier to trace issues Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

travis: require sudo b/c of sudo apt-...

b295827

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

fix checks location

a503103

TomasTomecek reviewed Jul 16, 2018

View reviewed changes



		def __init__(self, args, *kwargs):
		self._fmf_meta_init(args, kwargs)

Conversation

jscotka commented May 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

FMF Metadata works well

TODOs:

Colin output:

FMF output

when I want to be supercool and have backward compilant output:

Nosetests output - direct

using rulesets

advanced filtering

rulesets filters

Uh oh!

TomasTomecek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomasTomecek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomasTomecek commented Jul 2, 2018

Uh oh!

TomasTomecek commented Jul 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jscotka commented Jul 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jscotka commented May 31, 2018 •

edited

Loading