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 Cilium #17380

Merged
merged 1 commit into from
Apr 15, 2021
Merged

Add Cilium #17380

merged 1 commit into from
Apr 15, 2021

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Apr 1, 2021

This PR adds steps and workflows to enable testing with Cilium.

Steps and workflows are defined aws, azure & gcp. As Cilium requires additional ports, only an azure periodic is being added here for 4.8. Peridics for gcp & aws should be added in the future, but something will have to be done about Cilium ports.

@openshift-ci-robot
Copy link
Contributor

Hi @errordeveloper. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 1, 2021
@vrutkovs
Copy link
Member

vrutkovs commented Apr 1, 2021

copying from slack thread

Templates are deprecates, so ideally this would be converted into steps.
see https://docs.ci.openshift.org/docs/architecture/step-registry/

I think the best here would be:

In order to test this you could add a periodic job which runs this workflow

@errordeveloper errordeveloper changed the title template: Add cilium variant Add Cilium steps and workflows for aws, azure & gcp Apr 1, 2021
@errordeveloper
Copy link
Contributor Author

@vrutkovs I had a look around and figured that'd base this on structure similar to e.g. OVN rather then Loki, PTAL!

Copy link
Member

@vrutkovs vrutkovs left a comment

Choose a reason for hiding this comment

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

This looks good!

Please add this to the periodics (similar to #16278) so that rehearsals would give it a try

@vrutkovs
Copy link
Member

vrutkovs commented Apr 1, 2021

/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 1, 2021
@openshift-ci-robot openshift-ci-robot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Apr 1, 2021
@vrutkovs
Copy link
Member

vrutkovs commented Apr 1, 2021

AWS install is pretty strict about open ports, lets try azure instead?

@errordeveloper
Copy link
Contributor Author

@vrutkovs it definitely makes sense to prioritise Azure and make sure things work overall! I'll happily leave UPI and opening ports in GCP and AWS for another PR ;)

@vrutkovs
Copy link
Member

vrutkovs commented Apr 8, 2021

Lets add a new periodic (see https://github.com/openshift/release/pull/17519/files#diff-53da59c71bd31de3834afc38a2ccc62bf85595cb1546630ce3a10d4bfe6fe695) to have this rehearsed in CI (use make update to generate necessary configs)

@errordeveloper
Copy link
Contributor Author

 @vrutkovs once I add the periodic, will only run once the changes are merged, or it can be rehearsed from the PR branch somehow?

@vrutkovs
Copy link
Member

vrutkovs commented Apr 8, 2021

@vrutkovs once I add the periodic, will only run once the changes are merged, or it can be rehearsed from the PR branch somehow?

CI bot would add rehearse-... jobs to this PR. We won't merge the PR until it passes

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Apr 8, 2021
@errordeveloper errordeveloper force-pushed the cilium branch 3 times, most recently from 25a9239 to 605a30c Compare April 8, 2021 13:29
@errordeveloper
Copy link
Contributor Author

Closed accidentally, re-opening.

@errordeveloper
Copy link
Contributor Author

Few more just came up:

[sig-network] network isolation when using a plugin that does not isolate namespaces by default should allow communication between pods in different namespaces on different nodes

This is an OpenShift test.

[sig-network] Proxy version v1 should proxy logs on node with explicit kubelet port using proxy subresource [Suite:openshift/conformance/parallel] [Suite:k8s] expand_more

This is an upstream test.

 [sig-network] Networking Granular Checks: Services should function for endpoint-Service: udp [Suite:openshift/conformance/parallel] [Suite:k8s]

This one looks like it's upstream also.

Looks like these are flakes, neither have failed on the last attempt.

@errordeveloper
Copy link
Contributor Author

Looks like after renaming there is old rehearsal job still lurking around: ci/rehearse/periodic-ci-openshift-release-master-ci-4.8-e2e-azure-cilium-compact...

@errordeveloper
Copy link
Contributor Author

@vrutkovs I think this is actually good now, how do we go about the flakes? I'll squash the commits also.

- define steps and workflows for aws, azure & gcp
- define periodic job only for azure (gcp & aws will
  require some solution for Cilium ports)
@vrutkovs
Copy link
Member

I think we're good with leaving the flakes for now - we'll squash those after a few rounds of tests.

Lets update the description - why Azure, what this PR does (adds steps / workflows and a single periodic for 4.8) along with future plans.

/cc @deads2k @smarterclayton for review

@openshift-ci-robot
Copy link
Contributor

@vrutkovs: GitHub didn't allow me to request PR reviews from the following users: for, review.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

I think we're good with leaving the flakes for now - we'll squash those after a few rounds of tests.

Lets update the description - why Azure, what this PR does (adds steps / workflows and a single periodic for 4.8) along with future plans.

/cc @deads2k @smarterclayton for review

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vrutkovs
Copy link
Member

/cc @abhat @trozet for network parts

@openshift-ci-robot
Copy link
Contributor

@vrutkovs: GitHub didn't allow me to request PR reviews from the following users: for, parts.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @abhat @trozet for network parts

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@errordeveloper errordeveloper changed the title Add Cilium steps and workflows for aws, azure & gcp, and enable azure periodic Add Cilium Apr 12, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 12, 2021

@errordeveloper: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/rehearse/periodic-ci-openshift-release-master-ci-4.8-e2e-azure-cilium-compact 08528034b80fe3b473b4468a6475e6549c41601c link /test pj-rehearse
ci/rehearse/periodic-ci-openshift-release-master-ci-4.8-e2e-azure-cilium 70271bc link /test pj-rehearse
ci/prow/pj-rehearse 70271bc link /test pj-rehearse

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@errordeveloper
Copy link
Contributor Author

Looks like only one failure in the latest run:

[sig-arch] Managed cluster should have no crashlooping pods in core namespaces over four minutes [Suite:openshift/conformance/parallel]

cpu: 100m
memory: 100Mi
documentation: |-
This steps sets `networkType: None` and disables `cluster-network-operator` (CNO)
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@smarterclayton
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 15, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: errordeveloper, smarterclayton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 15, 2021
@openshift-merge-robot openshift-merge-robot merged commit 586120c into openshift:master Apr 15, 2021
@openshift-ci-robot
Copy link
Contributor

@errordeveloper: Updated the following 3 configmaps:

  • step-registry configmap in namespace ci at cluster app.ci using the following files:
    • key OWNERS using file ci-operator/step-registry/cilium/OWNERS
    • key OWNERS using file ci-operator/step-registry/cilium/conf/OWNERS
    • key cilium-conf-commands.sh using file ci-operator/step-registry/cilium/conf/cilium-conf-commands.sh
    • key cilium-conf-ref.metadata.json using file ci-operator/step-registry/cilium/conf/cilium-conf-ref.metadata.json
    • key cilium-conf-ref.yaml using file ci-operator/step-registry/cilium/conf/cilium-conf-ref.yaml
    • key OWNERS using file ci-operator/step-registry/network/OWNERS
    • key OWNERS using file ci-operator/step-registry/network/conf/OWNERS
    • key OWNERS using file ci-operator/step-registry/network/conf/disable-cno/OWNERS
    • key network-conf-disable-cno-commands.sh using file ci-operator/step-registry/network/conf/disable-cno/network-conf-disable-cno-commands.sh
    • key network-conf-disable-cno-ref.metadata.json using file ci-operator/step-registry/network/conf/disable-cno/network-conf-disable-cno-ref.metadata.json
    • key network-conf-disable-cno-ref.yaml using file ci-operator/step-registry/network/conf/disable-cno/network-conf-disable-cno-ref.yaml
    • key OWNERS using file ci-operator/step-registry/openshift/e2e/aws/cilium/OWNERS
    • key openshift-e2e-aws-cilium-workflow.metadata.json using file ci-operator/step-registry/openshift/e2e/aws/cilium/openshift-e2e-aws-cilium-workflow.metadata.json
    • key openshift-e2e-aws-cilium-workflow.yaml using file ci-operator/step-registry/openshift/e2e/aws/cilium/openshift-e2e-aws-cilium-workflow.yaml
    • key OWNERS using file ci-operator/step-registry/openshift/e2e/azure/cilium/OWNERS
    • key openshift-e2e-azure-cilium-workflow.metadata.json using file ci-operator/step-registry/openshift/e2e/azure/cilium/openshift-e2e-azure-cilium-workflow.metadata.json
    • key openshift-e2e-azure-cilium-workflow.yaml using file ci-operator/step-registry/openshift/e2e/azure/cilium/openshift-e2e-azure-cilium-workflow.yaml
    • key OWNERS using file ci-operator/step-registry/openshift/e2e/gcp/cilium/OWNERS
    • key openshift-e2e-gcp-cilium-workflow.metadata.json using file ci-operator/step-registry/openshift/e2e/gcp/cilium/openshift-e2e-gcp-cilium-workflow.metadata.json
    • key openshift-e2e-gcp-cilium-workflow.yaml using file ci-operator/step-registry/openshift/e2e/gcp/cilium/openshift-e2e-gcp-cilium-workflow.yaml
  • ci-operator-master-configs configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master__ci-4.8.yaml using file ci-operator/config/openshift/release/openshift-release-master__ci-4.8.yaml
  • job-config-master configmap in namespace ci at cluster app.ci using the following files:
    • key openshift-release-master-periodics.yaml using file ci-operator/jobs/openshift/release/openshift-release-master-periodics.yaml

In response to this:

This PR adds steps and workflows to enable testing with Cilium.

Steps and workflows are defined aws, azure & gcp. As Cilium requires additional ports, only an azure periodic is being added here for 4.8. Peridics for gcp & aws should be added in the future, but something will have to be done about Cilium ports.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants