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 ginkgo unit tests for configMaps #40

Merged
merged 4 commits into from
Mar 5, 2024

Conversation

Rizwana777
Copy link
Collaborator

@Rizwana777 Rizwana777 commented Feb 15, 2024

What does this PR do / why we need it:

  • Ginkgo tests for configMaps
  • minor updates in other test files

Which issue(s) this PR fixes:
Fixes #? Issue #22 and Issue #25 and #43

@Rizwana777 Rizwana777 changed the title Add ginkgo tests for congigMaps Add ginkgo tests for configMaps Feb 15, 2024
@Rizwana777 Rizwana777 changed the title Add ginkgo tests for configMaps Add ginkgo unit tests for configMaps Feb 15, 2024
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

In addition, I happened to notice that plugin download doesn't work on OpenShift (unfortunately we're not running the E2E tests against OpenShift, so it doesn't automatically catch this yet 😅). It would be good to fix it is as part of this PR. To reproduce:

  1. Log in to OpenShift cluster from clusterbot (e.g. rosa create 4.14)
  2. In rollouts manager dir, run hack/run-upstream-argo-rollouts-e2e-tests.sh

You will see it gets stuck on this step:

+ kubectl get -n argo-rollouts deployment/argo-rollouts
NAME            READY   UP-TO-DATE   AVAILABLE   AGE
argo-rollouts   0/1     1            0           0s
+ kubectl wait --for=condition=Available --timeout=10m -n argo-rollouts deployment/argo-rollouts

If you look at the logs of the argo-rollouts Pod, you will see the error:

[jgw@localhost-lan argo-rollouts-manager]$ k logs argo-rollouts-65b44974f6-6n9dl
time="2024-02-21T06:55:05Z" level=info msg="Argo Rollouts starting" version=v1.6.6+737ca89
time="2024-02-21T06:55:05Z" level=info msg="Using namespace argo-rollouts"
time="2024-02-21T06:55:05Z" level=info msg="GET https://172.30.0.1:443/api/v1/namespaces/argo-rollouts/secrets?labelSelector=istio.argoproj.io%2Fprimary-cluster%3Dtrue&limit=1 200 OK in 27 milliseconds\n"
time="2024-02-21T06:55:05Z" level=info msg="GET https://172.30.0.1:443/version 200 OK in 1 milliseconds\n"
time="2024-02-21T06:55:05Z" level=info msg="Creating event broadcaster"
time="2024-02-21T06:55:05Z" level=info msg="Setting up event handlers"
time="2024-02-21T06:55:05Z" level=info msg="Setting up experiments event handlers"
time="2024-02-21T06:55:05Z" level=info msg="Setting up analysis event handlers"
time="2024-02-21T06:55:05Z" level=info msg="GET https://172.30.0.1:443/api/v1/namespaces/argo-rollouts/configmaps/argo-rollouts-config 200 OK in 10 milliseconds\n"
time="2024-02-21T06:55:05Z" level=fatal msg="Failed to download plugins: failed to create plugin folder for plugin (argoproj-labs/openshift-route-plugin): (mkdir /home/argo-rollouts/plugin-bin: permission denied)"

To fix this, in deployment.go, you can probably modify our generation of the Deployment, so that the Deployment contains the following volumeMounts/volume fields:

kind: Deployment
  name: # (...)
  namespace: # (...)
spec:
  containers:
  - name: argo-rollouts
    volumeMounts:
    - mountPath: /home/argo-rollouts/plugin-bin
      name: tmp
  volumes:
  - emptyDir: {}
    name: tmp

controllers/default.go Outdated Show resolved Hide resolved
controllers/configmap_test.go Outdated Show resolved Hide resolved
controllers/configmap_test.go Outdated Show resolved Hide resolved
controllers/configmap_test.go Outdated Show resolved Hide resolved
controllers/configmap_test.go Outdated Show resolved Hide resolved
controllers/configmap_test.go Outdated Show resolved Hide resolved
controllers/deployment_test.go Outdated Show resolved Hide resolved
controllers/deployment_test.go Outdated Show resolved Hide resolved
Copy link
Member

@jgwest jgwest left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Rizwana777!

@jgwest jgwest merged commit 8ff476c into argoproj-labs:main Mar 5, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants