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

Improve tests #1667

Merged
merged 1 commit into from
May 31, 2019
Merged

Improve tests #1667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 3 additions & 6 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,17 +275,15 @@ jobs:
ARGOCD_FAKE_IN_CLUSTER: "true"
- run:
name: Start API server
command: go run ./cmd/argocd-server/main.go --loglevel debug --redis localhost:6379 --disable-auth --insecure --dex-server http://localhost:5556 --repo-server localhost:8081 --staticassets ../argo-cd-ui/dist/app
command: go run ./cmd/argocd-server/main.go --loglevel debug --redis localhost:6379 --insecure --dex-server http://localhost:5556 --repo-server localhost:8081 --staticassets ../argo-cd-ui/dist/app
background: true
environment:
ARGOCD_FAKE_IN_CLUSTER: "true"
- run:
name: Wait for API server
command: |
set -x
# TODO - try to reduce to 20
sleep 60
curl -v --retry 5 localhost:8080
until curl -v http://localhost:8080/healthz; do sleep 3; done
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

- run:
name: Start controller
command: go run ./cmd/argocd-application-controller/main.go --loglevel debug --redis localhost:6379 --repo-server localhost:8081 --kubeconfig ~/.kube/config
Expand All @@ -296,11 +294,10 @@ jobs:
name: Smoke test
command: |
set -x
argocd login localhost:8080 --plaintext --username admin --password password
argocd app create guestbook --dest-namespace default --dest-server https://kubernetes.default.svc --repo https://github.com/argoproj/argocd-example-apps.git --path guestbook
argocd app sync guestbook
argocd app delete guestbook
environment:
ARGOCD_OPTS: "--server localhost:8080 --plaintext"
- run:
name: Run e2e tests
command: |
Expand Down
13 changes: 6 additions & 7 deletions test/e2e/app_management_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,11 @@ func TestAppDeletion(t *testing.T) {
Delete(true).
Then().
Expect(DoesNotExist()).
Expect(Event(EventReasonResourceDeleted, "delete")).
And(func(_ *Application) {
// app should NOT be listed
output, err := fixture.RunCli("app", "list")
assert.NoError(t, err)
assert.NotContains(t, output, fixture.Name())
})
Expect(Event(EventReasonResourceDeleted, "delete"))

output, err := fixture.RunCli("app", "list")
assert.NoError(t, err)
assert.NotContains(t, output, fixture.Name())
}

func TestTrackAppStateAndSyncApp(t *testing.T) {
Expand Down Expand Up @@ -484,6 +482,7 @@ func TestPermissions(t *testing.T) {
proj.Spec.SourceRepos = []string{}
_, err = fixture.AppClientset.ArgoprojV1alpha1().AppProjects(fixture.ArgoCDNamespace).Update(proj)
assert.NoError(t, err)
time.Sleep(1 * time.Second)
closer, client, err := fixture.ArgoCDClientset.NewApplicationClient()
assert.NoError(t, err)
defer util.Close(closer)
Expand Down
13 changes: 2 additions & 11 deletions test/e2e/fixture/app/consequences.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package app

import (
"strings"
"time"

"github.com/ghodss/yaml"
log "github.com/sirupsen/logrus"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/argoproj/argo-cd/errors"
. "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
Expand Down Expand Up @@ -52,15 +51,7 @@ func (c *Consequences) app() *Application {
}

func (c *Consequences) get() (*Application, error) {
output, err := fixture.RunCli("app", "get", c.context.name, "-o", "yaml")
if err != nil {
if strings.Contains(output, "NotFound") {
return nil, nil
}
return nil, err
}
app := &Application{}
return app, yaml.Unmarshal([]byte(output), app)
return fixture.AppClientset.ArgoprojV1alpha1().Applications(fixture.ArgoCDNamespace).Get(c.context.name, v1.GetOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this looks odd, but the intent to was make sure that we prefer to invoke the CLI tool, than the API. How come you changed this bit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Forgot to mention it in PR description. This method is executed most frequently during e2e tests execution. Looks like any process fork takes at least 100ms, so I decided to switch it to api call to save several seconds

}

func (c *Consequences) resource(name string) ResourceStatus {
Expand Down
9 changes: 5 additions & 4 deletions test/e2e/fixture/app/expectation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

v1 "k8s.io/api/core/v1"
apierr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"

Expand Down Expand Up @@ -83,13 +84,13 @@ func ResourceHealthIs(resource string, expected HealthStatusCode) Expectation {

func DoesNotExist() Expectation {
return func(c *Consequences) (state, string) {
app, err := c.get()
_, err := c.get()
if err != nil {
if apierr.IsNotFound(err) {
return succeeded, "app does not exist"
}
return failed, err.Error()
}
if app == nil {
return succeeded, "app does not exist"
}
return pending, "app should not exist"
}
}
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/fixture/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fixture

import (
"os"
"os/exec"
"strings"
"time"
Expand All @@ -15,6 +16,7 @@ func Run(workDir, name string, args ...string) (string, error) {
log.WithFields(log.Fields{"name": name, "args": args, "workDir": workDir}).Info("running command")

cmd := exec.Command(name, args...)
cmd.Env = os.Environ()
cmd.Dir = workDir

outBytes, err := cmd.Output()
Expand Down
35 changes: 15 additions & 20 deletions test/e2e/fixture/fixture.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
jsonpatch "github.com/evanphx/json-patch"
"github.com/ghodss/yaml"
log "github.com/sirupsen/logrus"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
"k8s.io/client-go/tools/clientcmd"
Expand Down Expand Up @@ -63,10 +64,6 @@ func getKubeConfig(configPath string, overrides clientcmd.ConfigOverrides) *rest
// creates e2e tests fixture: ensures that Application CRD is installed, creates temporal namespace, starts repo and api server,
// configure currently available cluster.
func init() {

// trouble-shooting check to see if this busted add-on is going to cause problems
FailOnErr(Run("", "kubectl", "api-resources", "-o", "name"))
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


// set-up variables
config := getKubeConfig("", clientcmd.ConfigOverrides{})
AppClientset = appclientset.NewForConfigOrDie(config)
Expand Down Expand Up @@ -127,31 +124,28 @@ func CreateSecret(username, password string) string {
secretName := fmt.Sprintf("argocd-e2e-secret-%s", id)
FailOnErr(Run("", "kubectl", "create", "secret", "generic", secretName,
"--from-literal=username="+username,
"--from-literal=password="+password))
FailOnErr(Run("", "kubectl", "label", "secret", secretName, testingLabel+"=true"))
"--from-literal=password="+password,
"-n", ArgoCDNamespace))
FailOnErr(Run("", "kubectl", "label", "secret", secretName, testingLabel+"=true", "-n", ArgoCDNamespace))
return secretName
}

func EnsureCleanState() {

start := time.Now()

policy := v1.DeletePropagationBackground
// delete resources
text, err := Run("", "kubectl", "get", "app", "-o", "name")
CheckError(err)
for _, name := range strings.Split(text, "\n") {
if name != "" {
appName := strings.TrimPrefix(name, "application.argoproj.io/")
// we cannot delete any app, if an op is in progress
_, _ = RunCli("app", "terminate-op", appName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to maintain this line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no need to terminate operation after fixing #1665. After application is deleted the controller is no longs stuck trying to update operation state.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// is it much more reliable to get argocd to delete an app than kubectl, deleting directly can make ArgoCD stuck
_, _ = RunCli("app", "delete", appName)
}
}
FailOnErr(Run("", "kubectl", "delete", "appprojects", "--field-selector", "metadata.name!=default"))
// takes around 5s, so we don't wait
// kubectl delete apps --all
CheckError(AppClientset.ArgoprojV1alpha1().Applications(ArgoCDNamespace).DeleteCollection(&v1.DeleteOptions{PropagationPolicy: &policy}, v1.ListOptions{}))
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe comment with the kubectl equivilant?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure. added

// kubectl delete appprojects --field-selector metadata.name!=default
CheckError(AppClientset.ArgoprojV1alpha1().AppProjects(ArgoCDNamespace).DeleteCollection(
&v1.DeleteOptions{PropagationPolicy: &policy}, v1.ListOptions{FieldSelector: "metadata.name!=default"}))
// kubectl delete secrets -l e2e.argoproj.io=true
CheckError(KubeClientset.CoreV1().Secrets(ArgoCDNamespace).DeleteCollection(
&v1.DeleteOptions{PropagationPolicy: &policy}, v1.ListOptions{LabelSelector: testingLabel + "=true"}))

FailOnErr(Run("", "kubectl", "delete", "ns", "-l", testingLabel+"=true", "--field-selector", "status.phase=Active", "--wait=false"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to delete this line too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We still need to deleted temporal namespaces. There is no API to delete collection of namespaces, so I kept one kubectl delete ... . Would you prefer to use api here for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nope, not fussed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm.. How can there be no API for this, I'd assumed that kubectl just makes API calls?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you are right. there is API, but no API to delete collection, only one by one deletion

FailOnErr(Run("", "kubectl", "delete", "secrets", "-l", testingLabel+"=true"))

// reset settings
s, err := SettingsManager.GetSettings()
Expand All @@ -160,6 +154,7 @@ func EnsureCleanState() {
// changing theses causes a restart
AdminPasswordHash: s.AdminPasswordHash,
AdminPasswordMtime: s.AdminPasswordMtime,
ServerSignature: s.ServerSignature,
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Certificate: s.Certificate,
DexConfig: s.DexConfig,
OIDCConfigRAW: s.OIDCConfigRAW,
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/repo_creds_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ func TestCanAddAppFromPrivateRepoWithCredConfig(t *testing.T) {
Path(appPath).
And(func() {
secretName := fixture.CreateSecret("blah", accessToken)
FailOnErr(fixture.Run("", "kubectl", "patch", "cm", "argocd-cm", "-p", fmt.Sprintf(`{"data": {"repository.credentials": "- passwordSecret:\n key: password\n name: %s\n url: %s\n usernameSecret:\n key: username\n name: %s\n"}}`, secretName, repoUrl, secretName)))
FailOnErr(fixture.Run("", "kubectl", "patch", "cm", "argocd-cm",
"-n", fixture.ArgoCDNamespace,
"-p", fmt.Sprintf(`{"data": {"repository.credentials": "- passwordSecret:\n key: password\n name: %s\n url: %s\n usernameSecret:\n key: username\n name: %s\n"}}`, secretName, repoUrl, secretName)))
}).
When().
Create().
Expand Down