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.

Sign up for GitHub

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

Modify operator hub tests to run in separate namespaces #3239

Merged
merged 5 commits into from
May 27, 2020
Merged

Modify operator hub tests to run in separate namespaces #3239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5710eae
Modify operator hub tests to run in separate namespaces
dharmit May 22, 2020
b118f62
Changes as per PR review
dharmit May 22, 2020
b0ff12e
Remove createEtcdClusterService function and more separate specs
dharmit May 22, 2020
a683722
Loop for two operators
dharmit May 22, 2020
488d66f
Make operator hub tests sequential
dharmit May 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,4 +339,4 @@ openshiftci-presubmit-unittests:
# Run OperatorHub tests
.PHONY: test-operator-hub
test-operator-hub:
ginkgo $(GINKGO_FLAGS) -focus="odo service command tests" tests/integration/operatorhub/
ginkgo $(GINKGO_FLAGS_SERIAL) -focus="odo service command tests" tests/integration/operatorhub/
20 changes: 3 additions & 17 deletions scripts/setup-operators.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ install_etcd_operator(){
kind: Subscription
metadata:
name: etcd
namespace: ${OPERATOR_HUB_PROJECT}
namespace: openshift-operators
Copy link
Member Author

@dharmit dharmit May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Earlier, etcd operator was being installed in CI_OPERATOR_HUB_PROJECT and mongodb operator in openshift-operators. The way this was done made etcd operator available only in the CI_OPERATOR_HUB_PROJECT namespace while mongodb operator was available across the cluster.

Now, I have modified the code to install etcd operator across the cluster as well. The way this works is, we install it in openshift-operators namespace using clusterwide-alpha channel. This makes sure that the installed operator is available across all the namespaces on the cluster. Serves the purpose of both installing an operator in single namespce as well as across the cluster.

dharmit marked this conversation as resolved.
Show resolved Hide resolved
spec:
channel: singlenamespace-alpha
channel: clusterwide-alpha
installPlanApproval: Automatic
name: etcd
source: community-operators
Expand All @@ -51,25 +51,11 @@ do
fi
done

# Now onto namespace bound operator
# Create OperatorGroup
oc create -f - <<EOF
dharmit marked this conversation as resolved.
Show resolved Hide resolved
apiVersion: operators.coreos.com/v1
kind: OperatorGroup
metadata:
generateName: ${CI_OPERATOR_HUB_PROJECT}-
generation: 1
namespace: ${CI_OPERATOR_HUB_PROJECT}
spec:
targetNamespaces:
- ${CI_OPERATOR_HUB_PROJECT}
EOF

# install etcd operator
count=0
while [ "$count" -lt "5" ];
do
if oc get csv -n ${CI_OPERATOR_HUB_PROJECT} | grep etcd; then
if oc get csv -n openshift-operators | grep etcd; then
break
else
install_etcd_operator
Expand Down
158 changes: 103 additions & 55 deletions tests/integration/operatorhub/cmd_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package integration
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"regexp"
"strings"
Expand All @@ -14,21 +13,45 @@ import (
"github.com/openshift/odo/tests/helper"
)

const (
CI_OPERATOR_HUB_PROJECT = "ci-operator-hub-project"
)

var _ = Describe("odo service command tests for OperatorHub", func() {

var project string

BeforeEach(func() {
SetDefaultEventuallyTimeout(10 * time.Minute)
SetDefaultConsistentlyDuration(30 * time.Second)
helper.CmdShouldPass("odo", "project", "set", CI_OPERATOR_HUB_PROJECT)
// TODO: remove this when OperatorHub integration is fully baked into odo
os.Setenv("ODO_EXPERIMENTAL", "true")
helper.CmdShouldPass("odo", "preference", "set", "Experimental", "true")
Copy link
Contributor

Choose a reason for hiding this comment

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

Preference will clash for each spec. SO you need to overwrite the preference path each time the spec is called. For example

context = helper.CreateNewContext()
os.Setenv("GLOBALODOCONFIG", filepath.Join(context, "config.yaml"))
helper.CmdShouldPass("odo", "preference", "set", "Experimental", "true")

Copy link
Contributor

Choose a reason for hiding this comment

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

Make sure you are un setting the environment variable GLOBALODOCONFIG in AfterEach teardown step too

})

preSetup := func() {
project = helper.CreateRandProject()
helper.CmdShouldPass("odo", "project", "set", project)

// wait till oc can see the all operators installed by setup script in the namespace
ocArgs := []string{"get", "csv"}
operators := []string{"etcd", "mongodb"}
for _, operator := range operators {
helper.WaitForCmdOut("oc", ocArgs, 1, true, func(output string) bool {
return strings.Contains(output, operator)
})
}
}

cleanPreSetup := func() {
helper.DeleteProject(project)
}

Context("When experimental mode is enabled", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("should list operators installed in the namespace", func() {
stdOut := helper.CmdShouldPass("odo", "catalog", "list", "services")
Expect(stdOut).To(ContainSubstring("Operators available in the cluster"))
Expand All @@ -38,18 +61,26 @@ var _ = Describe("odo service command tests for OperatorHub", func() {
})

Context("When creating an operator backed service", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("should be able to create EtcdCluster from its alm example", func() {
// First let's grab the etcd operator's name from "odo catalog list services" output
operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]`).FindString(operators)

etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]-clusterwide`).FindString(operators)
helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster")

pods := helper.CmdShouldPass("oc", "get", "pods", "-n", CI_OPERATOR_HUB_PROJECT)
// now verify if the pods for the operator have started
pods := helper.CmdShouldPass("oc", "get", "pods", "-n", project)
// Look for pod with example name because that's the name etcd will give to the pods.
etcdPod := regexp.MustCompile(`example-.[a-z0-9]*`).FindString(pods)

ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", CI_OPERATOR_HUB_PROJECT}
ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", project}
helper.WaitForCmdOut("oc", ocArgs, 1, true, func(output string) bool {
return strings.Contains(output, "Running")
})
Expand All @@ -62,10 +93,19 @@ var _ = Describe("odo service command tests for OperatorHub", func() {
})

Context("When using dry-run option to create operator backed service", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("should only output the definition of the CR that will be used to start service", func() {
// First let's grab the etcd operator's name from "odo catalog list services" output
operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]`).FindString(operators)
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]-clusterwide`).FindString(operators)
Copy link
Member Author

Choose a reason for hiding this comment

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

This modification is done because earlier when etcd operator was installed in a single namespace, output of odo catalog list services would be as below:

odo catalog list services
Operators available in the cluster
NAME                                CRDs
etcdoperator.v0.9.4     EtcdCluster, EtcdBackup, EtcdRestore

But since it's now being installed from clusterwide channel, it looks like:

odo catalog list services
Operators available in the cluster
NAME                                CRDs
etcdoperator.v0.9.4-clusterwide     EtcdCluster, EtcdBackup, EtcdRestore


stdOut := helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster", "--dry-run")
Expect(stdOut).To(ContainSubstring("apiVersion"))
Expand All @@ -74,22 +114,22 @@ var _ = Describe("odo service command tests for OperatorHub", func() {
})

Context("When using from-file option", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("should be able to create a service", func() {
// First let's grab the etcd operator's name from "odo catalog list services" output
operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]`).FindString(operators)
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]-clusterwide`).FindString(operators)

stdOut := helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster", "--dry-run")

// change the metadata.name from example to example2 so that we can run tests parallely
lines := strings.Split(stdOut, "\n")
for i, line := range lines {
if strings.Contains(line, "name: example") {
lines[i] = strings.Replace(lines[i], "example", "example2", 1)
}
}
stdOut = strings.Join(lines, "\n")

// stdOut contains the yaml specification. Store it to a file
randomFileName := helper.RandString(6) + ".yaml"
fileName := filepath.Join("/tmp", randomFileName)
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work on windows. Please use context instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not a part of the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I understood but It can be fixed right? It's not a big change that we can go for a new pr and review process. Anyway in the commit message you can mention the fix so that we can find out what this pr fixes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not checked if it's a big change or a small one. Last time I tried to fix something in my PR that had not been touched in my PR, it ended up going for a really long time. So, if you think that this line of code is problematic, I humbly request you to open a new issue. But I'm not addressing it in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue has been tracked in #3246

Expand All @@ -101,19 +141,30 @@ var _ = Describe("odo service command tests for OperatorHub", func() {
helper.CmdShouldPass("odo", "service", "create", "--from-file", fileName)

// now verify if the pods for the operator have started
pods := helper.CmdShouldPass("oc", "get", "pods", "-n", CI_OPERATOR_HUB_PROJECT)
pods := helper.CmdShouldPass("oc", "get", "pods", "-n", project)
Copy link
Contributor

Choose a reason for hiding this comment

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

As we are already setting project using odo command, There is no use of explicitly mentioning -n flag here. So I did just a check to confirm how odo project set sets active projects for oc too.

$ odo project create xya
 ✓  Project 'xya' is ready for use
 ✓  New project created and now using project: xya
$ odo project list
ACTIVE     NAME
           default
           [...]
           viraj
*          xya
$ oc projects
You have access to the following projects and can switch between them with 'oc project <projectname>':

    default
    [...]
    viraj
  * xya
$ odo project set viraj
Switched to project : viraj
$ odo project list
ACTIVE     NAME
           default
           [...]
*          viraj
           xya
$ oc projects
You have access to the following projects and can switch between them with 'oc project <projectname>':

    default
    [...]
  * viraj
    xya

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm just being explicit here. Besides, in this comment #3193 (comment) you have mentioned to provide the project name for better user experience. So can you please decide if you want to use --project or not?

Copy link
Contributor

@prietyc123 prietyc123 May 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It seems you misunderstood my comment. NP, I have put the details in the issue what was trying to say.

As part of the --project flag is concerned TBH it depends on how you are going to use this. Here you have already setting the active project through odo project set... that too in justBeforeEach. So it really does not make sense to pass the --project flag in the spec implementation.

If you want to use the --project flag then don't use set command. Either way implementation will work fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems you misunderstood my comment. NP, I have put the details in the issue what was trying to say.

This comment thread which started from #3239 (comment) is about "not using the -n flag in tests", whereas that issue is about "project name not showing up in the output". How are the two things similar? How did I misunderstand what you were trying to say? What am I missing here? 🤔

As part of the --project flag is concerned TBH it depends on how you are going to use this. Here you have already setting the active project through odo project set... that too in justBeforeEach. So it really does not make sense to pass the --project flag in the spec implementation.

If you want to use the --project flag then don't use set command. Either way implementation will work fine.

This part of your comment is about what you started out with in #3239 (comment). But the issue and this comment thread don't seem to have anything in common !

// Look for pod with example name because that's the name etcd will give to the pods.
etcdPod := regexp.MustCompile(`example2-.[a-z0-9]*`).FindString(pods)
etcdPod := regexp.MustCompile(`example-.[a-z0-9]*`).FindString(pods)

ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", CI_OPERATOR_HUB_PROJECT}
ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", project}
helper.WaitForCmdOut("oc", ocArgs, 1, true, func(output string) bool {
return strings.Contains(output, "Running")
})

// Delete the pods created. This should idealy be done by `odo
// service delete` but that's not implemented for operator backed
// services yet.
helper.CmdShouldPass("oc", "delete", "EtcdCluster", "example2")
helper.CmdShouldPass("oc", "delete", "EtcdCluster", "example")
dharmit marked this conversation as resolved.
Show resolved Hide resolved
})
})

Context("When using from-file option", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("should fail to create service if metadata doesn't exist or is invalid", func() {
Expand Down Expand Up @@ -159,6 +210,15 @@ spec:
})

Context("JSON output", func() {

JustBeforeEach(func() {
preSetup()
})

JustAfterEach(func() {
cleanPreSetup()
})

It("listing catalog of services", func() {
jsonOut := helper.CmdShouldPass("odo", "catalog", "list", "services", "-o", "json")
Expect(jsonOut).To(ContainSubstring("mongodb-enterprise"))
Expand All @@ -167,54 +227,42 @@ spec:
})

Context("When operator backed services are created", func() {
It("should list the services if they exist", func() {
// First let's grab the etcd operator's name from "odo catalog list services" output
operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]`).FindString(operators)

stdOut := helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster", "--dry-run")

// change the metadata.name from example to example3 so that we can run tests parallely
lines := strings.Split(stdOut, "\n")
for i, line := range lines {
if strings.Contains(line, "name: example") {
lines[i] = strings.Replace(lines[i], "example", "example3", 1)
}
}
stdOut = strings.Join(lines, "\n")
JustBeforeEach(func() {
preSetup()
})

// stdOut contains the yaml specification. Store it to a file
randomFileName := helper.RandString(6) + ".yaml"
fileName := filepath.Join("/tmp", randomFileName)
if err := ioutil.WriteFile(fileName, []byte(stdOut), 0644); err != nil {
fmt.Printf("Could not write yaml spec to file %s because of the error %v", fileName, err.Error())
}
JustAfterEach(func() {
cleanPreSetup()
})

// now create operator backed service
helper.CmdShouldPass("odo", "service", "create", "--from-file", fileName)
It("should list the services if they exist", func() {
operators := helper.CmdShouldPass("odo", "catalog", "list", "services")
etcdOperator := regexp.MustCompile(`etcdoperator\.*[a-z][0-9]\.[0-9]\.[0-9]-clusterwide`).FindString(operators)
helper.CmdShouldPass("odo", "service", "create", etcdOperator, "--crd", "EtcdCluster")

// now verify if the pods for the operator have started
pods := helper.CmdShouldPass("oc", "get", "pods", "-n", CI_OPERATOR_HUB_PROJECT)
pods := helper.CmdShouldPass("oc", "get", "pods", "-n", project)
// Look for pod with example name because that's the name etcd will give to the pods.
etcdPod := regexp.MustCompile(`example3-.[a-z0-9]*`).FindString(pods)
etcdPod := regexp.MustCompile(`example-.[a-z0-9]*`).FindString(pods)

ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", CI_OPERATOR_HUB_PROJECT}
ocArgs := []string{"get", "pods", etcdPod, "-o", "template=\"{{.status.phase}}\"", "-n", project}
helper.WaitForCmdOut("oc", ocArgs, 1, true, func(output string) bool {
return strings.Contains(output, "Running")
})

stdOut = helper.CmdShouldPass("odo", "service", "list")
stdOut := helper.CmdShouldPass("odo", "service", "list")
Expect(stdOut).To(ContainSubstring("example"))
Expect(stdOut).To(ContainSubstring("EtcdCluster"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Make it in a single check as you have done in https://github.com/openshift/odo/pull/3239/files#diff-49f09fb5341717455c0421b3c592e685R243

Copy link
Member Author

Choose a reason for hiding this comment

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

Not touched in this PR. Please open a separate issue for this to be addressed. I can't have double standards between this and #3239 (comment) even if I'm aware of the change that needs to be made. I'm sorry.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dharmit Do you need a separate issue for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue #3245


// now check for json output
jsonOut := helper.CmdShouldPass("odo", "service", "list", "-o", "json")
helper.MatchAllInOutput(jsonOut, []string{"\"apiVersion\": \"etcd.database.coreos.com/v1beta2\"", "\"kind\": \"EtcdCluster\"", "\"name\": \"example3\""})
helper.MatchAllInOutput(jsonOut, []string{"\"apiVersion\": \"etcd.database.coreos.com/v1beta2\"", "\"kind\": \"EtcdCluster\"", "\"name\": \"example\""})

// Delete the pods created. This should idealy be done by `odo
// service delete` but that's not implemented for operator backed
// services yet.
helper.CmdShouldPass("oc", "delete", "EtcdCluster", "example3")
helper.CmdShouldPass("oc", "delete", "EtcdCluster", "example")

// Now let's check the output again to ensure expected behaviour
stdOut = helper.CmdShouldFail("odo", "service", "list")
Expand Down