e2e-tests: auto generate advanced molecule ansible tests with samples… #4312

camilamacedo86 · 2020-12-14T16:02:29Z

Description

Replace static mock test/ansible by auto-generated mock via samples stack

Motivation for the change:

Ensure that e2e molecule tests are done with the PR changes by automating its generation. Otherwise, we cannot check if a change in the scaffold files could break it.

Closes: #4025

Note: To check the scenario locally run go run ./hack/generate/samples/molecule/generate.go the advanced-molecule-operator will be generate in the testdata. Then, it can be checked for development purposes.

**Description of the change:** - update advanced mock static scenario with 1.0+ layout (#3433) - (note that we need to ensure that the test works with the new layout) - Fix CI issue (blocker master): `The error was: urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='172.30.99.1', port=24443): Max retries exceeded with url: /version (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f28647cdb38>: Failed to establish a new connection: [Errno 110] Connection timed out',))` in GA. NOTE: The follow up here is to replace the test/ansible static mock with samples to ensure that we are testing it with the latest changes always. See the issue #4025 and its PR : #4312

jmrodri

There is a typo. But in general what if this PR doesn't merge? While I understand what we're trying to do, I'm not 100% sure if this easier to maintain.

jmrodri · 2020-12-18T16:20:38Z

hack/tests/e2e-ansible-molecule.sh


-header_text "Test Ansible Molecule scenarios"
-pushd "${ROOTDIR}/test/ansible"
+header_text "Running Defualt test with advanced-molecule-operator/"


typo should read "Default"

hack/generate/samples/internal/ansible/advanced_molecule.go

camilamacedo86 · 2020-12-18T16:45:17Z

Hi @jmrodri.

Just to clarifies. We are not replacing here a shell script by a go stack.
Currently, we have a mock project in the test/ansible which would requires manually updates and checkes in all reviews after any change in the scaffolds. Indeed if it is a bump of a new version of Kubebuilder be made since it could impact that as well.

Then, this PR solves this problem. After that, we are generating automatically the sample used for molecule tests. See that the e2e molecule runs 2 tests. One is based on a Memcached project with its specific scenarios which is automated by the go samples stack already. The other one runs using as mock the /test/ansible.

But in general what if this PR doesn't merge? While I understand what we're trying to do, I'm not 100% sure if this easier to maintain.

It makes it very easier since is :

we are following here the same approach applied accross the project to generate the samples + to build the other mock data used by the e2e molecule tests.
we no longer need to check and update the test/ansible after any change made

PS.: We still able to just run the molecule generate go file and check the testdata created for development purpose as local tests as well. See the makefile call. (go run ./hack/generate/samples/molecule/generate.go)

All your suggestions are addressed as well. Really tks.

jmrodri · 2020-12-18T16:48:57Z

@camilamacedo86 okay. That makes a lot of sense.

jmrodri

/lgtm

asmacdo · 2020-12-18T18:12:12Z

.gitignore

@@ -20,6 +20,7 @@ website/tech-doc-hugo

 # Ignore molecule samples testdata if it be generated in the testdata/ diretory
 testdata/ansible/memcached-molecule-operator
+testdata/ansible/advanced-molecule-operator


+1 good name

asmacdo · 2020-12-18T18:12:43Z

hack/generate/samples/internal/ansible/advanced_molecule.go

+	"github.com/operator-framework/operator-sdk/internal/testutils"
+)
+
+// MoleculeAnsible defines the context for the sample


Suggested change

// MoleculeAnsible defines the context for the sample

// AdcancedMolecule defines the context for the sample

asmacdo · 2020-12-18T18:16:11Z

hack/generate/samples/internal/ansible/advanced_molecule.go

+      k8s_info:
+        api_version: v1
+        kind: ConfigMap
+        namespace: '{{ meta.namespace }}'


should this not be ansible_operator_meta like the other one? https://github.com/operator-framework/operator-sdk/blob/master/testdata/ansible/memcached-operator/roles/memcached/tasks/main.yml#L9

No. Because we pass in the watches file the var meta. see;

operator-sdk/hack/generate/samples/internal/ansible/testdata/watches.yaml

Lines 57 to 62 in 4d5b883

- version: v1alpha1

group: test.example.com

kind: ReconciliationTest

playbook: playbooks/reconciliationtest.yml

vars:

meta: '{{ ansible_operator_meta }}'

hmm. IMO we ought to remove that and just access ansible_operator_meta directly. That said, it does not need to be in this PR, just something I noticed.

asmacdo

/lgtm

**Description of the change:** - update advanced mock static scenario with 1.0+ layout (operator-framework#3433) - (note that we need to ensure that the test works with the new layout) - Fix CI issue (blocker master): `The error was: urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='172.30.99.1', port=24443): Max retries exceeded with url: /version (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f28647cdb38>: Failed to establish a new connection: [Errno 110] Connection timed out',))` in GA. NOTE: The follow up here is to replace the test/ansible static mock with samples to ensure that we are testing it with the latest changes always. See the issue operator-framework#4025 and its PR : operator-framework#4312 Signed-off-by: reinvantveer <[email protected]>

operator-framework#4312) **Description** Replace static mock test/ansible by auto-generated mock via samples stack **Motivation for the change:** Ensure that e2e molecule tests are done with the PR changes by automating its generation. Otherwise, we cannot check if a change in the scaffold files could break it. Closes: operator-framework#4025 **Note**: To check the scenario locally run ` go run ./hack/generate/samples/molecule/generate.go` the advanced-molecule-operator will be generate in the testdata. Then, it can be checked for development purposes. Signed-off-by: reinvantveer <[email protected]>

**Description of the change:** - update advanced mock static scenario with 1.0+ layout (operator-framework#3433) - (note that we need to ensure that the test works with the new layout) - Fix CI issue (blocker master): `The error was: urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='172.30.99.1', port=24443): Max retries exceeded with url: /version (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7f28647cdb38>: Failed to establish a new connection: [Errno 110] Connection timed out',))` in GA. NOTE: The follow up here is to replace the test/ansible static mock with samples to ensure that we are testing it with the latest changes always. See the issue operator-framework#4025 and its PR : operator-framework#4312 Signed-off-by: rearl <[email protected]>

operator-framework#4312) **Description** Replace static mock test/ansible by auto-generated mock via samples stack **Motivation for the change:** Ensure that e2e molecule tests are done with the PR changes by automating its generation. Otherwise, we cannot check if a change in the scaffold files could break it. Closes: operator-framework#4025 **Note**: To check the scenario locally run ` go run ./hack/generate/samples/molecule/generate.go` the advanced-molecule-operator will be generate in the testdata. Then, it can be checked for development purposes. Signed-off-by: rearl <[email protected]>

openshift-ci-robot requested review from estroz and fabianvf December 14, 2020 16:02

camilamacedo86 changed the title ~~e2e-tests: auto generate advanced molecule ansible tests with samples…~~ WIP: e2e-tests: auto generate advanced molecule ansible tests with samples… Dec 14, 2020

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 14, 2020

camilamacedo86 mentioned this pull request Dec 14, 2020

e2e-tests: fix advanced molecule ansible tests #4273

Merged

camilamacedo86 changed the title ~~WIP: e2e-tests: auto generate advanced molecule ansible tests with samples…~~ e2e-tests: auto generate advanced molecule ansible tests with samples… Dec 16, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2020

camilamacedo86 requested review from jmrodri and asmacdo December 16, 2020 01:03

camilamacedo86 mentioned this pull request Dec 16, 2020

The file config/rbac/role.yaml has not been updated with the APIs which are created #4310

Closed

jmrodri requested changes Dec 18, 2020

View reviewed changes

jmrodri approved these changes Dec 18, 2020

View reviewed changes

openshift-ci-robot assigned jmrodri Dec 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2020

e2e-tests: auto generate advanced molecule ansible tests

0250ea5

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2020

latest layout fmt

4d5b883

asmacdo reviewed Dec 18, 2020

View reviewed changes

fix typo austin review

1cd9150

asmacdo approved these changes Dec 18, 2020

View reviewed changes

openshift-ci-robot assigned asmacdo Dec 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2020

camilamacedo86 merged commit 50fcdb8 into operator-framework:master Dec 18, 2020

camilamacedo86 deleted the mocule-samples branch December 18, 2020 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

e2e-tests: auto generate advanced molecule ansible tests with samples… #4312

e2e-tests: auto generate advanced molecule ansible tests with samples… #4312

camilamacedo86 commented Dec 14, 2020 •

edited

Loading

jmrodri left a comment

jmrodri Dec 18, 2020

camilamacedo86 Dec 18, 2020

camilamacedo86 commented Dec 18, 2020 •

edited

Loading

jmrodri commented Dec 18, 2020

jmrodri left a comment

asmacdo Dec 18, 2020

asmacdo Dec 18, 2020

camilamacedo86 Dec 18, 2020

asmacdo Dec 18, 2020

camilamacedo86 Dec 18, 2020

asmacdo Dec 18, 2020

asmacdo left a comment

	// MoleculeAnsible defines the context for the sample
	// AdcancedMolecule defines the context for the sample

	- version: v1alpha1
	group: test.example.com
	kind: ReconciliationTest
	playbook: playbooks/reconciliationtest.yml
	vars:
	meta: '{{ ansible_operator_meta }}'

e2e-tests: auto generate advanced molecule ansible tests with samples… #4312

e2e-tests: auto generate advanced molecule ansible tests with samples… #4312

Conversation

camilamacedo86 commented Dec 14, 2020 • edited Loading

jmrodri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

camilamacedo86 commented Dec 18, 2020 • edited Loading

jmrodri commented Dec 18, 2020

jmrodri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asmacdo left a comment

Choose a reason for hiding this comment

camilamacedo86 commented Dec 14, 2020 •

edited

Loading

camilamacedo86 commented Dec 18, 2020 •

edited

Loading