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

openstack/nfv: update experimental job #27145

Merged
merged 1 commit into from
Mar 22, 2022
Merged

openstack/nfv: update experimental job #27145

merged 1 commit into from
Mar 22, 2022

Conversation

EmilienM
Copy link
Member

Use the experimental NFV job to test the improvements made into the
virtual plugin for the SR-IOV Network Operator.

  • Add DPDK to the networkpolicy
  • Change how the testpmd pod is being deployed to simplify things and
    not relying on manual CNI edits.

@openshift-ci openshift-ci bot requested review from mandre and MaysaMacedo March 21, 2022 16:27
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 21, 2022
Use the experimental NFV job to test the improvements made into the
virtual plugin for the SR-IOV Network Operator.

* Add DPDK to the networkpolicy
* Change how the testpmd pod is being deployed to simplify things and
  not relying on manual CNI edits.
@EmilienM
Copy link
Member Author

/retest

@@ -4,8 +4,6 @@ workflow:
pre:
- chain: ipi-openstack-pre
- ref: openstack-provision-performanceprofile
- ref: openstack-provision-vfio-noiommu
Copy link
Contributor

Choose a reason for hiding this comment

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

this is being removed because we're loading from the kernelargs now right?
k8snetworkplumbingwg/sriov-network-operator#257
if yes, do we plan to remove from the other workflows later?

Copy link
Member Author

@EmilienM EmilienM Mar 22, 2022

Choose a reason for hiding this comment

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

this is being removed because we're loading from the kernelargs now right?

Yes, once Sebastian's PR to support vhostuser has landed, we can remove openstack-provision-vfio-noiommu because the SR-IOV network operator will take care of it.

if yes, do we plan to remove from the other workflows later?

Absolutely! But first we use the experimental workflow to test things on demand, before changing the "stable" periodic jobs that use the "legacy" manual steps.

Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

sure. I thought this step openstack-provision-vfio-noiommu could already be excluded since this was merged k8snetworkplumbingwg/sriov-network-operator#257


cat <<EOF > "${SHARED_DIR}/additionalnetwork.yaml"
if oc get SriovNetworkNodePolicy/dpdk1 -n openshift-sriov-network-operator >/dev/null 2>&1; then
echo "DPDK network is managed by SriovNetworkNodePolicy/dpdk1, CNI won't be changed"
Copy link
Contributor

@MaysaMacedo MaysaMacedo Mar 22, 2022

Choose a reason for hiding this comment

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

So a Pod created on that host will already have the new interface configured correctly witthout the CNI configure with host-device?

I think we need update on the README https://github.com/openshift/sriov-network-operator/blob/184af61a5406c2c6298eb88c10303d90d5266c76/README.md#sriovnetworknodepolicy.
Let me know what you think and I will try to do.

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 a Pod but a SriovNetworkNodePolicy.

See:

$ oc describe SriovNetworkNodePolicy/dpdk1 -n openshift-sriov-network-operator
Name:         dpdk1
Namespace:    openshift-sriov-network-operator
Labels:       <none>
Annotations:  <none>
API Version:  sriovnetwork.openshift.io/v1
Kind:         SriovNetworkNodePolicy
Metadata:
  Creation Timestamp:  2022-03-21T18:27:37Z
  Generation:          1
  Managed Fields:
    API Version:  sriovnetwork.openshift.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:spec:
        .:
        f:deviceType:
        f:nicSelector:
          .:
          f:netFilter:
        f:nodeSelector:
          .:
          f:feature.node.kubernetes.io/network-sriov.capable:
        f:numVfs:
        f:priority:
        f:resourceName:
    Manager:         kubectl-create
    Operation:       Update
    Time:            2022-03-21T18:27:37Z
  Resource Version:  61357
  UID:               b61924be-aea1-4286-88e9-9fadb991dd88
Spec:
  Device Type:  vfio-pci
  Is Rdma:      false
  Nic Selector:
    Net Filter:  openstack/NetworkID:4303a5b3-3406-4044-b19f-c11a18c075bd
  Node Selector:
    feature.node.kubernetes.io/network-sriov.capable:  true
  Num Vfs:                                             1
  Priority:                                            99
  Resource Name:                                       dpdk1
Events:                                                <none>

We have this condition to detect whether or not we created a SriovNetworkNodePolicy for DPDK, and if not, then we'll patch additionalNetworks in Cluster Network Operator.

Does it make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. However, why the CNI settings are not needed anymore when the SriovNetworkNodePolicy is available? I'm just wondering at which step the Pod will be configured with the new nic

@EmilienM
Copy link
Member Author

Note for reviewers: e2e-openstack-nfv-experimental is expected to fail; and it'll start passing once we land some patches into the SR-IOV network operator.

@EmilienM
Copy link
Member Author

The errors are weird, i wonder if the infra is flacky or under load.
/retest

@MaysaMacedo
Copy link
Contributor

/lgtm
/hold for the ci results

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: EmilienM, MaysaMacedo

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

@MaysaMacedo
Copy link
Contributor

/retest

@MaysaMacedo
Copy link
Contributor

/test pj-rehearse

@MaysaMacedo
Copy link
Contributor

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2022
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 8145d00 into openshift:master Mar 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 22, 2022

@EmilienM: Updated the step-registry configmap in namespace ci at cluster app.ci using the following files:

  • key openshift-e2e-openstack-nfv-experimental-workflow.yaml using file ci-operator/step-registry/openshift/e2e/openstack/nfv-experimental/openshift-e2e-openstack-nfv-experimental-workflow.yaml
  • key openstack-provision-sriov-networknodepolicy-commands.sh using file ci-operator/step-registry/openstack/provision/sriov-networknodepolicy/openstack-provision-sriov-networknodepolicy-commands.sh
  • key openstack-provision-sriov-networknodepolicy-ref.yaml using file ci-operator/step-registry/openstack/provision/sriov-networknodepolicy/openstack-provision-sriov-networknodepolicy-ref.yaml
  • key openstack-test-dpdk-commands.sh using file ci-operator/step-registry/openstack/test/dpdk/openstack-test-dpdk-commands.sh

In response to this:

Use the experimental NFV job to test the improvements made into the
virtual plugin for the SR-IOV Network Operator.

  • Add DPDK to the networkpolicy
  • Change how the testpmd pod is being deployed to simplify things and
    not relying on manual CNI edits.

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
Copy link
Contributor

openshift-ci bot commented Mar 22, 2022

@EmilienM: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/rehearse/periodic-ci-shiftstack-shiftstack-ci-main-periodic-4.9-e2e-openstack-nfv-mellanox 03baffc71895a96724a1d03cb12f7c302b784daa link unknown /test pj-rehearse
ci/rehearse/periodic-ci-shiftstack-shiftstack-ci-main-periodic-4.9-e2e-openstack-nfv-intel 03baffc71895a96724a1d03cb12f7c302b784daa link unknown /test pj-rehearse
ci/rehearse/periodic-ci-shiftstack-shiftstack-ci-main-periodic-4.11-e2e-openstack-nfv-intel 2e3e0be link unknown /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.

@EmilienM EmilienM deleted the exp_update branch March 22, 2022 17:20
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants