Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add integration tests #6354

Merged
merged 3 commits into from Aug 14, 2019

Conversation

lgarciaaco
Copy link
Contributor

@lgarciaaco lgarciaaco commented Aug 12, 2019

This PR creates the infrastructure for e2e tests for the operator and adds two basic integration tests. installWithDefaultsTest test the installation of sydensis when an empty CR is created. installTest uses a populated CR.

Did some cleanup.

Closes #6353

@pure-bot pure-bot bot added the pr/review-requested Use this if you want to have a review. pure-bot will prevent merging if set and no review given label Aug 12, 2019
Luis Garcia Acosta and others added 2 commits August 12, 2019 13:47
Co-Authored-By: Zoran Regvart <zoran@regvart.com>
Copy link
Member

@zregvart zregvart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering what happens to the syndesis-operator role creation when cluster doesn't have some of the resources that the role references installed (Camel K, Integreatly CRs?)

The tests look at the status of the Syndesis CR to assert it was installed, should we put more asserts towards asserting that deployment configs or other resources are created?

kind: Role
apiVersion: rbac.authorization.k8s.io/v1beta1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changed from rbac.authorization.k8s.io/v1 to rbac.authorization.k8s.io/v1beta1, should it have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as the next comment

@@ -1,11 +1,16 @@
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1
apiVersion: rbac.authorization.k8s.io/v1beta1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be?

Suggested change
apiVersion: rbac.authorization.k8s.io/v1beta1
apiVersion: rbac.authorization.k8s.io/v1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you probably right, I blindly copy from

apiVersion: rbac.authorization.k8s.io/v1beta1
, maybe @chirino can shed some lights

labels:
app: syndesis
syndesis.io/app: syndesis
syndesis.io/type: operator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps like this, to distinguish from the operator? Not sure what labels we set on other service accounts...

Suggested change
syndesis.io/type: operator
syndesis.io/type: service-account

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to refers to the components the system account is managing, for instance:

- apiVersion: v1
kind: ServiceAccount
metadata:
name: syndesis-server
labels:
app: syndesis
syndesis.io/app: syndesis
syndesis.io/type: infrastructure
syndesis.io/component: syndesis-server

or

- apiVersion: v1
kind: ServiceAccount
metadata:
name: syndesis-default
labels:
app: syndesis
syndesis.io/app: syndesis
syndesis.io/type: infrastructure
syndesis.io/component: syndesis-infrastructure

so I guess operator is acceptable, but not sure whether we have some guidelines for it, this was again a c&p from:

package e2e

import (
goctx "context"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does go-fmt like this? I thought we were using tabs in Golang code

@lgarciaaco
Copy link
Contributor Author

lgarciaaco commented Aug 13, 2019

@zregvart

I'm wondering what happens to the syndesis-operator role creation when cluster doesn't have some of the resources that the role references installed (Camel K, Integreatly CRs?)

The test suite uses the resources in https://github.com/syndesisio/syndesis/tree/master/install/operator/deploy, if the CRD is missing it will attempt to create it. This tests only need syndesis CRDs atm, I guess as we incorporate more tests that use other CRDs (like kamel), we would have to move those tho the crd folder

The tests look at the status of the Syndesis CR to assert it was installed, should we put more asserts towards asserting that deployment configs or other resources are created?

Definitely!! This is just the bare structure to start adding more on top of that, the fun is just starting ;)

@lgarciaaco lgarciaaco merged commit 65edab7 into syndesisio:master Aug 14, 2019
@lgarciaaco lgarciaaco deleted the operator-integration-tests branch August 14, 2019 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/review-requested Use this if you want to have a review. pure-bot will prevent merging if set and no review given
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create e2e and integration test for operator
3 participants