fix flake8 errors in Kubespray CI - tox-inventory-builder #6910

hafe · 2020-11-15T09:50:08Z

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug

/kind cleanup

/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Kubespray CI - tox-inventory-builder no longer works

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

k8s-ci-robot · 2020-11-15T09:50:16Z

Hi @hafe. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

oomichi · 2020-11-16T00:21:21Z

For example, https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/849333077 shows the following error:

pep8 run-test: commands[0] | bash -c 'find /builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/* -type f -name '"'"'*.py'"'"' -print0 | xargs -0 flake8'
/builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/tests/test_inventory.py:54:13: H214: Use assertIn/NotIn(A, B) rather than assertTrue/False(A in/not in B) when checking collection contents.
            self.assertTrue(group in self.inv.yaml_config['all']['children'])
            ^
/builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/tests/test_inventory.py:230:9: H214: Use assertIn/NotIn(A, B) rather than assertTrue/False(A in/not in B) when checking collection contents.
        self.assertTrue(
        ^
/builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/tests/test_inventory.py:249:13: H214: Use assertIn/NotIn(A, B) rather than assertTrue/False(A in/not in B) when checking collection contents.
            self.assertTrue(
            ^
/builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/tests/test_inventory.py:258:9: H214: Use assertIn/NotIn(A, B) rather than assertTrue/False(A in/not in B) when checking collection contents.
        self.assertTrue(
        ^
/builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/tests/test_inventory.py:266:9: H214: Use assertIn/NotIn(A, B) rather than assertTrue/False(A in/not in B) when checking collection contents.
        self.assertTrue(
        ^
ERROR: InvocationError for command /bin/bash -c 'find /builds/kargo-ci/kubernetes-sigs-kubespray/contrib/inventory_builder/* -type f -name '"'"'*.py'"'"' -print0 | xargs -0 flake8' (exited with code 123)

Nice fixing.

/lgtm

oomichi · 2020-11-16T00:23:11Z

/ok-to-test

hafe · 2020-11-16T06:42:29Z

/retest

electrocucaracha

Few suggestions

electrocucaracha · 2020-11-16T17:53:19Z

contrib/inventory_builder/tests/test_inventory.py

        for group in groups:
-            self.assertTrue(group in self.inv.yaml_config['all']['children'])
+            self.assertIn(group, self.inv.yaml_config['all']['children'])


What about something that doesn't iterate over the groups like:

self.assertTrue(set(groups).issuperset(set(self.inv.yaml_config['all']['children'])))

electrocucaracha · 2020-11-16T18:03:48Z

contrib/inventory_builder/tests/test_inventory.py

-        self.assertTrue(
-            bad_host not in self.inv.yaml_config['all']['hosts'].keys())
+        self.assertNotIn(
+            bad_host, self.inv.yaml_config['all']['hosts'].keys())


I think that this works without having to specify the keys() method

Suggested change

bad_host, self.inv.yaml_config['all']['hosts'].keys())

bad_host, self.inv.yaml_config['all']['hosts'])

electrocucaracha · 2020-11-16T18:05:05Z

contrib/inventory_builder/tests/test_inventory.py

@@ -246,25 +246,25 @@ def test_set_k8s_cluster(self):

        self.inv.set_k8s_cluster()
        for host in expected_hosts:
-            self.assertTrue(
-                host in
+            self.assertIn(


Same suggestion as the first comment

self.assertTrue(set(expected_hosts).issuperset(self.inv.yaml_config['all']['children'][group]['children']))

hafe · 2020-11-16T18:32:41Z

Few suggestions

I just changed what was suggested in ci. I don't even have a clue what this piece of code is there for...

hafe · 2020-11-16T19:19:27Z

/retest

oomichi · 2020-11-16T22:41:20Z

I just changed what was suggested in ci. I don't even have a clue what this piece of code is there for...

Yeah I agree with @hafe here.
It is better to concentrate on how to fix the CI issue based on the error messages at this time.

hafe · 2020-11-17T07:26:35Z

/retest

hafe · 2020-11-21T18:43:22Z

/retest

k8s-ci-robot · 2020-11-22T20:34:04Z

@bmelbourne: The /retest command does not accept any targets.
The following commands are available to trigger jobs:

/test pull-kubespray-yamllint

Use /test all to run all jobs.

In response to this:

/retest tf-ovh_ubuntu18-calico

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2020-11-22T20:37:29Z

@bmelbourne: The specified target(s) for /test were not found.
The following commands are available to trigger jobs:

/test pull-kubespray-yamllint

Use /test all to run all jobs.

In response to this:

/test tf-ovh_ubuntu18-calico

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bmelbourne · 2020-11-22T20:38:14Z

/retest

bmelbourne · 2020-11-22T21:13:48Z

/lgtm
/approve

oomichi · 2020-11-23T02:12:17Z

/lgtm

floryut

At last 👍
/approve

k8s-ci-robot · 2020-11-23T07:47:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bmelbourne, floryut, hafe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [floryut]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…-sigs#6910) * fix flake8 errors in Kubespray CI - tox-inventory-builder * Invalidate CRI-O kubic repo's cache Signed-off-by: Victor Morales <v.morales@samsung.com> * add support to configure pkg install retries and use in CI job tf-ovh_ubuntu18-calico (due to it failing often) * Switch Calico, Cilium and MetalLB image repos to Quay.io Co-authored-by: Victor Morales <v.morales@samsung.com> Co-authored-by: Barry Melbourne <9964974+bmelbourne@users.noreply.github.com>

k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 15, 2020

k8s-ci-robot requested review from holmsten and oomichi November 15, 2020 09:50

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 15, 2020

k8s-ci-robot assigned oomichi Nov 16, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2020

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 16, 2020

hafe mentioned this pull request Nov 16, 2020

Update assert for py test following tox update #6913

Closed

electrocucaracha reviewed Nov 16, 2020

View reviewed changes

fix flake8 errors in Kubespray CI - tox-inventory-builder

bbc8818

hafe force-pushed the fix-tox-inventory-builder branch from dfbea02 to ec9dba8 Compare November 18, 2020 10:01

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 18, 2020

hafe force-pushed the fix-tox-inventory-builder branch from ec9dba8 to 8b50326 Compare November 18, 2020 11:31

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 18, 2020

hafe force-pushed the fix-tox-inventory-builder branch from 8b50326 to 5d0b568 Compare November 18, 2020 13:08

floryut mentioned this pull request Nov 18, 2020

Issue 6925 inventory builder tests failing #6926

Closed

hafe force-pushed the fix-tox-inventory-builder branch from 5d0b568 to c548706 Compare November 18, 2020 15:22

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 18, 2020

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 21, 2020

bmelbourne mentioned this pull request Nov 22, 2020

Upgrade Jetstack Cert-Manager v1.0.4 #6937

Merged

Switch Calico, Cilium and MetalLB image repos to Quay.io

817754d

hafe mentioned this pull request Nov 22, 2020

Switch Calico, Cilium and MetalLB image repos to Quay.io #6942

Closed

k8s-ci-robot assigned bmelbourne Nov 22, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 22, 2020

floryut approved these changes Nov 23, 2020

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2020

k8s-ci-robot merged commit ee23b94 into kubernetes-sigs:master Nov 23, 2020

champtar mentioned this pull request Dec 11, 2020

[2.14] fix ci #7021

Merged

RickHaan mentioned this pull request Jan 8, 2021

[2.13] Backport CI fix #7119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix flake8 errors in Kubespray CI - tox-inventory-builder #6910

fix flake8 errors in Kubespray CI - tox-inventory-builder #6910

hafe commented Nov 15, 2020

k8s-ci-robot commented Nov 15, 2020

oomichi commented Nov 16, 2020

oomichi commented Nov 16, 2020

hafe commented Nov 16, 2020

electrocucaracha left a comment

electrocucaracha Nov 16, 2020

electrocucaracha Nov 16, 2020

electrocucaracha Nov 16, 2020

hafe commented Nov 16, 2020

hafe commented Nov 16, 2020

oomichi commented Nov 16, 2020

hafe commented Nov 17, 2020

hafe commented Nov 21, 2020

k8s-ci-robot commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

bmelbourne commented Nov 22, 2020

bmelbourne commented Nov 22, 2020

oomichi commented Nov 23, 2020

floryut left a comment

k8s-ci-robot commented Nov 23, 2020

	bad_host, self.inv.yaml_config['all']['hosts'].keys())
	bad_host, self.inv.yaml_config['all']['hosts'])

fix flake8 errors in Kubespray CI - tox-inventory-builder #6910

fix flake8 errors in Kubespray CI - tox-inventory-builder #6910

Conversation

hafe commented Nov 15, 2020

k8s-ci-robot commented Nov 15, 2020

oomichi commented Nov 16, 2020

oomichi commented Nov 16, 2020

hafe commented Nov 16, 2020

electrocucaracha left a comment

Choose a reason for hiding this comment

electrocucaracha Nov 16, 2020

Choose a reason for hiding this comment

electrocucaracha Nov 16, 2020

Choose a reason for hiding this comment

electrocucaracha Nov 16, 2020

Choose a reason for hiding this comment

hafe commented Nov 16, 2020

hafe commented Nov 16, 2020

oomichi commented Nov 16, 2020

hafe commented Nov 17, 2020

hafe commented Nov 21, 2020

k8s-ci-robot commented Nov 22, 2020

k8s-ci-robot commented Nov 22, 2020

bmelbourne commented Nov 22, 2020

bmelbourne commented Nov 22, 2020

oomichi commented Nov 23, 2020

floryut left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 23, 2020