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 multi control plane e2e tests #610

Merged
merged 3 commits into from
Feb 6, 2025

Conversation

luksa
Copy link
Contributor

@luksa luksa commented Feb 4, 2025

No description provided.

@luksa luksa requested a review from a team as a code owner February 4, 2025 09:36
@luksa luksa force-pushed the multi-control-plane-test branch from f9089bd to 89f1f82 Compare February 4, 2025 09:38
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.27%. Comparing base (bbc577d) to head (47eeeb9).
Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #610      +/-   ##
==========================================
- Coverage   74.43%   74.27%   -0.16%     
==========================================
  Files          40       40              
  Lines        2511     2511              
==========================================
- Hits         1869     1865       -4     
- Misses        554      557       +3     
- Partials       88       89       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@luksa luksa force-pushed the multi-control-plane-test branch from 89f1f82 to 65e2d85 Compare February 4, 2025 10:21
@luksa luksa force-pushed the multi-control-plane-test branch from 65e2d85 to 3535aac Compare February 4, 2025 10:33
@luksa
Copy link
Contributor Author

luksa commented Feb 4, 2025

/retest

@sridhargaddam
Copy link
Contributor

The tests are failing due to docker rate-limiting.

    2m16s       Warning   Failed                         pod/istiod-7c5bf57d85-hjnpt      Failed to pull image "docker.io/istio/pilot:1.24.2-distroless": failed to pull and unpack image "docker.io/istio/pilot:1.24.2-distroless": failed to copy: httpReadSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/istio/pilot/manifests/sha256:137e44e3d1d2af7421b6817b2e913cd4be8602e8e6a30f7e1c90614923926771: 429 Too Many Requests - Server message: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit

@luksa
Copy link
Contributor Author

luksa commented Feb 5, 2025

/retest

1 similar comment
@luksa
Copy link
Contributor Author

luksa commented Feb 5, 2025

/retest

Copy link
Contributor

@fjglira fjglira left a comment

Choose a reason for hiding this comment

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

lgtm only small comment to make more clear to the people reading the test

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("Multiple Control Planes", Ordered, func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it will be easier to understand if it said: "Multiple Control Planes Installation"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this isn't testing the just the installation of multiple control planes. It's testing the "entire" multi-control plane scenario.

})

It("Verifies app2a cannot connect to app1", func(ctx SpecContext) {
// time.Sleep(10 * time.Minute)
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this?

Copy link
Contributor

Choose a reason for hiding this comment

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

And maybe add a note here:

Note: Is expected to fail the request between apps off different namespace and meshes

Copy link
Contributor

Choose a reason for hiding this comment

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

Or better, add a Success( Request between namespaces on the different mesh are failing as expected)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

output, err := k.WithNamespace(appNamespace2a).
Exec("deploy/sleep", "sleep", fmt.Sprintf("curl -sIL http://httpbin.%s:8000", appNamespace2b))
Expect(err).NotTo(HaveOccurred(), "error running curl in sleep pod")
Expect(output).To(ContainSubstring("200 OK"), fmt.Sprintf("Unexpected response from sleep pod in namespace %s", appNamespace2b))
Copy link
Contributor

@fjglira fjglira Feb 5, 2025

Choose a reason for hiding this comment

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

Add a Success( Request between applications of the same mesh are successfully)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@luksa luksa force-pushed the multi-control-plane-test branch from 5ed1c31 to bae66ad Compare February 6, 2025 07:06
cl, err = k8sclient.InitK8sClient("")
Expect(err).NotTo(HaveOccurred())

k = kubectl.New("clControlPlane")
Copy link
Contributor

Choose a reason for hiding this comment

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

To differentiate between the controlPlane testSuite which is already using "clControlPlane".

Suggested change
k = kubectl.New("clControlPlane")
k = kubectl.New("clMultiControlPlane")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was wondering about this. Why is this called "clSomething"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind. I looked at why this is needed and have now removed it from most of the tests. This is only needed for tests involving multiple clusters.

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("Multi control plane deployment model", Ordered, func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the other testSuites, we run the tests against all the supported versions. However, in this PR we are validating it with supportedversion.New alone. Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it was intentional. I started with only a single version, because if we want to test this on multiple versions, then we actually need to test it on all possible version pair combinations. Do you think we should do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be an overkill to test all possible combinations. I am fine with validating against a single version.

@luksa luksa force-pushed the multi-control-plane-test branch from ad1111f to 2bffd8c Compare February 6, 2025 08:09
@luksa luksa force-pushed the multi-control-plane-test branch from 2bffd8c to 47eeeb9 Compare February 6, 2025 08:13
@istio-testing istio-testing merged commit 14dcddb into istio-ecosystem:main Feb 6, 2025
15 of 16 checks passed
FilipB added a commit to FilipB/sail-operator that referenced this pull request Feb 7, 2025
* upstream-eco/main: (27 commits)
  Add documentation for version alias (istio-ecosystem#625)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#624)
  Capture ztunnel details on e2e failures (istio-ecosystem#618)
  [Doc] Updates to dual-stack section (istio-ecosystem#621)
  Add multi control plane e2e tests (istio-ecosystem#610)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#620)
  Add workflow to detect broken links in doc (istio-ecosystem#611)
  Add an e2e test for Ambient config in IstioCNI (istio-ecosystem#591)
  Use gcr.io for Istio images and quay.io/sail-dev for sample applications (istio-ecosystem#613)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#612)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#609)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#608)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#606)
  Helm: Remove redundant platform-specific cmd during installation (istio-ecosystem#604)
  add trigger for nightly builds of release-1.0
  bump version to 1.1.0
  Update ztunnel comment in api_transformer (istio-ecosystem#595)
  Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#599)
  link ambient doc in the doc/table of contents (istio-ecosystem#598)
  Add an Ambient mode documentation (istio-ecosystem#579)
  ...

# Conflicts:
#	bundle/manifests/sailoperator.clusterserviceversion.yaml
#	resources/latest/charts/base/Chart.yaml
#	resources/latest/charts/cni/Chart.yaml
#	resources/latest/charts/cni/values.yaml
#	resources/latest/charts/gateway/Chart.yaml
#	resources/latest/charts/gateway/templates/deployment.yaml
#	resources/latest/charts/istiod/Chart.yaml
#	resources/latest/charts/istiod/values.yaml
#	resources/latest/charts/revisiontags/values.yaml
#	resources/latest/charts/ztunnel/Chart.yaml
#	resources/latest/charts/ztunnel/values.yaml
#	resources/latest/profiles/ambient.yaml
#	resources/latest/profiles/default.yaml
#	resources/latest/profiles/demo.yaml
#	resources/latest/profiles/empty.yaml
#	resources/latest/profiles/openshift-ambient.yaml
#	resources/latest/profiles/openshift.yaml
#	resources/latest/profiles/preview.yaml
#	resources/latest/profiles/remote.yaml
#	resources/latest/profiles/stable.yaml
#	resources/v1.21.6/charts/cni/values.yaml
#	resources/v1.21.6/charts/istiod/values.yaml
#	resources/v1.21.6/charts/revisiontags/values.yaml
#	resources/v1.21.6/charts/ztunnel/values.yaml
#	resources/v1.21.6/profiles/ambient.yaml
#	resources/v1.21.6/profiles/default.yaml
#	resources/v1.21.6/profiles/demo.yaml
#	resources/v1.21.6/profiles/empty.yaml
#	resources/v1.21.6/profiles/external.yaml
#	resources/v1.21.6/profiles/openshift.yaml
#	resources/v1.21.6/profiles/preview.yaml
#	resources/v1.21.6/profiles/remote.yaml
#	resources/v1.22.5/charts/cni/values.yaml
#	resources/v1.22.5/charts/istiod/values.yaml
#	resources/v1.22.5/charts/revisiontags/values.yaml
#	resources/v1.22.5/charts/ztunnel/values.yaml
#	resources/v1.22.5/profiles/ambient.yaml
#	resources/v1.22.5/profiles/default.yaml
#	resources/v1.22.5/profiles/demo.yaml
#	resources/v1.22.5/profiles/empty.yaml
#	resources/v1.22.5/profiles/openshift-ambient.yaml
#	resources/v1.22.5/profiles/openshift.yaml
#	resources/v1.22.5/profiles/preview.yaml
#	resources/v1.22.5/profiles/remote.yaml
#	resources/v1.22.5/profiles/stable.yaml
#	resources/v1.22.6/charts/cni/values.yaml
#	resources/v1.22.6/charts/istiod/values.yaml
#	resources/v1.22.6/charts/revisiontags/values.yaml
#	resources/v1.22.6/charts/ztunnel/values.yaml
#	resources/v1.22.6/profiles/ambient.yaml
#	resources/v1.22.6/profiles/default.yaml
#	resources/v1.22.6/profiles/demo.yaml
#	resources/v1.22.6/profiles/empty.yaml
#	resources/v1.22.6/profiles/openshift-ambient.yaml
#	resources/v1.22.6/profiles/openshift.yaml
#	resources/v1.22.6/profiles/preview.yaml
#	resources/v1.22.6/profiles/remote.yaml
#	resources/v1.22.6/profiles/stable.yaml
#	resources/v1.22.7/charts/cni/values.yaml
#	resources/v1.22.7/charts/istiod/values.yaml
#	resources/v1.22.7/charts/revisiontags/values.yaml
#	resources/v1.22.7/charts/ztunnel/values.yaml
#	resources/v1.22.7/profiles/ambient.yaml
#	resources/v1.22.7/profiles/default.yaml
#	resources/v1.22.7/profiles/demo.yaml
#	resources/v1.22.7/profiles/empty.yaml
#	resources/v1.22.7/profiles/openshift-ambient.yaml
#	resources/v1.22.7/profiles/openshift.yaml
#	resources/v1.22.7/profiles/preview.yaml
#	resources/v1.22.7/profiles/remote.yaml
#	resources/v1.22.7/profiles/stable.yaml
#	resources/v1.22.8/charts/cni/values.yaml
#	resources/v1.22.8/charts/istiod/values.yaml
#	resources/v1.22.8/charts/revisiontags/values.yaml
#	resources/v1.22.8/charts/ztunnel/values.yaml
#	resources/v1.22.8/profiles/ambient.yaml
#	resources/v1.22.8/profiles/default.yaml
#	resources/v1.22.8/profiles/demo.yaml
#	resources/v1.22.8/profiles/empty.yaml
#	resources/v1.22.8/profiles/openshift-ambient.yaml
#	resources/v1.22.8/profiles/openshift.yaml
#	resources/v1.22.8/profiles/preview.yaml
#	resources/v1.22.8/profiles/remote.yaml
#	resources/v1.22.8/profiles/stable.yaml
#	resources/v1.23.2/charts/cni/values.yaml
#	resources/v1.23.2/charts/istiod-remote/values.yaml
#	resources/v1.23.2/charts/istiod/values.yaml
#	resources/v1.23.2/charts/revisiontags/values.yaml
#	resources/v1.23.2/charts/ztunnel/values.yaml
#	resources/v1.23.2/profiles/ambient.yaml
#	resources/v1.23.2/profiles/default.yaml
#	resources/v1.23.2/profiles/demo.yaml
#	resources/v1.23.2/profiles/empty.yaml
#	resources/v1.23.2/profiles/openshift-ambient.yaml
#	resources/v1.23.2/profiles/openshift.yaml
#	resources/v1.23.2/profiles/preview.yaml
#	resources/v1.23.2/profiles/remote.yaml
#	resources/v1.23.2/profiles/stable.yaml
#	resources/v1.23.3/charts/cni/values.yaml
#	resources/v1.23.3/charts/istiod-remote/values.yaml
#	resources/v1.23.3/charts/istiod/values.yaml
#	resources/v1.23.3/charts/revisiontags/values.yaml
#	resources/v1.23.3/charts/ztunnel/values.yaml
#	resources/v1.23.3/profiles/ambient.yaml
#	resources/v1.23.3/profiles/default.yaml
#	resources/v1.23.3/profiles/demo.yaml
#	resources/v1.23.3/profiles/empty.yaml
#	resources/v1.23.3/profiles/openshift-ambient.yaml
#	resources/v1.23.3/profiles/openshift.yaml
#	resources/v1.23.3/profiles/preview.yaml
#	resources/v1.23.3/profiles/remote.yaml
#	resources/v1.23.3/profiles/stable.yaml
#	resources/v1.23.4/charts/cni/values.yaml
#	resources/v1.23.4/charts/istiod-remote/values.yaml
#	resources/v1.23.4/charts/istiod/values.yaml
#	resources/v1.23.4/charts/revisiontags/values.yaml
#	resources/v1.23.4/charts/ztunnel/values.yaml
#	resources/v1.23.4/profiles/ambient.yaml
#	resources/v1.23.4/profiles/default.yaml
#	resources/v1.23.4/profiles/demo.yaml
#	resources/v1.23.4/profiles/empty.yaml
#	resources/v1.23.4/profiles/openshift-ambient.yaml
#	resources/v1.23.4/profiles/openshift.yaml
#	resources/v1.23.4/profiles/preview.yaml
#	resources/v1.23.4/profiles/remote.yaml
#	resources/v1.23.4/profiles/stable.yaml
#	resources/v1.24.0/charts/cni/values.yaml
#	resources/v1.24.0/charts/istiod/values.yaml
#	resources/v1.24.0/charts/revisiontags/values.yaml
#	resources/v1.24.0/charts/ztunnel/values.yaml
#	resources/v1.24.0/profiles/ambient.yaml
#	resources/v1.24.0/profiles/default.yaml
#	resources/v1.24.0/profiles/demo.yaml
#	resources/v1.24.0/profiles/empty.yaml
#	resources/v1.24.0/profiles/openshift-ambient.yaml
#	resources/v1.24.0/profiles/openshift.yaml
#	resources/v1.24.0/profiles/preview.yaml
#	resources/v1.24.0/profiles/remote.yaml
#	resources/v1.24.0/profiles/stable.yaml
#	resources/v1.24.1/charts/cni/values.yaml
#	resources/v1.24.1/charts/istiod/values.yaml
#	resources/v1.24.1/charts/revisiontags/values.yaml
#	resources/v1.24.1/charts/ztunnel/values.yaml
#	resources/v1.24.1/profiles/ambient.yaml
#	resources/v1.24.1/profiles/default.yaml
#	resources/v1.24.1/profiles/demo.yaml
#	resources/v1.24.1/profiles/empty.yaml
#	resources/v1.24.1/profiles/openshift-ambient.yaml
#	resources/v1.24.1/profiles/openshift.yaml
#	resources/v1.24.1/profiles/preview.yaml
#	resources/v1.24.1/profiles/remote.yaml
#	resources/v1.24.1/profiles/stable.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants