diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 0000000000..e8813c67b4 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,11 @@ +self-hosted-runner: + labels: + - ubuntu-22.04-4core + - ubuntu-22.04-64core + - ubuntu-22.04-8core + - ubuntu-22.04-arm-4core + - ubuntu-24.04-4core + - ubuntu-24.04-8core + - ubuntu-24.04-arm-4core + - sriov + - ocp diff --git a/.github/workflows/actionlint.yaml b/.github/workflows/actionlint.yaml new file mode 100644 index 0000000000..f244939ec5 --- /dev/null +++ b/.github/workflows/actionlint.yaml @@ -0,0 +1,26 @@ +name: Actionlint + +on: + pull_request: + paths: + - .github/workflows/** + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: write + pull-requests: write + +jobs: + actionlint: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Run Actionlint + uses: raven-actions/actionlint@v2 + with: + files: .github/workflows/*.y*ml diff --git a/.github/workflows/chart-push-release.yml b/.github/workflows/chart-push-release.yml index 912ed8fb08..185558b560 100644 --- a/.github/workflows/chart-push-release.yml +++ b/.github/workflows/chart-push-release.yml @@ -1,4 +1,4 @@ -name: "Push helm chart on release" +name: Push helm chart on release env: IMAGE_NAME: ghcr.io/${{ github.repository }} diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 2c827b1bb4..e7b9de8744 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,12 +1,12 @@ -name: "CodeQL" +name: CodeQL on: push: - branches: [ "master" ] + branches: [ master ] pull_request: - branches: [ "master" ] + branches: [ master ] schedule: - - cron: "48 5 * * 6" + - cron: 48 5 * * 6 jobs: analyze: @@ -24,18 +24,18 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Initialize CodeQL - uses: github/codeql-action/init@v2 + uses: github/codeql-action/init@v3 with: languages: ${{ matrix.language }} queries: +security-and-quality - name: Autobuild - uses: github/codeql-action/autobuild@v2 + uses: github/codeql-action/autobuild@v3 - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v2 + uses: github/codeql-action/analyze@v3 with: category: "/language:${{ matrix.language }}" diff --git a/.github/workflows/image-push-master.yml b/.github/workflows/image-push-master.yml index 90ac6d48ab..dad8735d57 100644 --- a/.github/workflows/image-push-master.yml +++ b/.github/workflows/image-push-master.yml @@ -1,4 +1,4 @@ -name: "Push images on merge to master" +name: Push images on merge to master env: IMAGE_NAME: ghcr.io/${{ github.repository }} @@ -11,10 +11,10 @@ on: jobs: build-and-push-image-operator: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -22,10 +22,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -33,12 +33,12 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }} - name: Build and push sriov-network-operator - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true @@ -50,10 +50,10 @@ jobs: file: ./Dockerfile build-and-push-image-config-daemon: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -61,10 +61,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -72,12 +72,12 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }}-config-daemon - name: Build and push sriov-network-operator-config-daemon - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true @@ -89,10 +89,10 @@ jobs: file: ./Dockerfile.sriov-network-config-daemon build-and-push-image-webhook: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -100,10 +100,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -111,12 +111,12 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }}-webhook - name: Build and push sriov-network-operator-webhook - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true diff --git a/.github/workflows/image-push-release.yml b/.github/workflows/image-push-release.yml index 031a8ed52a..87a9f5a83c 100644 --- a/.github/workflows/image-push-release.yml +++ b/.github/workflows/image-push-release.yml @@ -1,4 +1,4 @@ -name: "Push images on release" +name: Push images on release env: IMAGE_NAME: ghcr.io/${{ github.repository }} @@ -10,10 +10,10 @@ on: - v* jobs: build-and-push-image-operator: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -21,10 +21,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -32,14 +32,14 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }} flavor: | latest=false - name: Build and push sriov-network-operator - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true @@ -50,10 +50,10 @@ jobs: file: ./Dockerfile build-and-push-image-config-daemon: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -61,10 +61,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -72,14 +72,14 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }}-config-daemon flavor: | latest=false - name: Build and push sriov-network-operator-config-daemon - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true @@ -90,10 +90,10 @@ jobs: file: ./Dockerfile.sriov-network-config-daemon build-and-push-image-webhook: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Add support for more platforms with QEMU (optional) # https://github.com/docker/setup-qemu-action @@ -101,10 +101,10 @@ jobs: uses: docker/setup-qemu-action@v3 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v2 + uses: docker/setup-buildx-action@v3 - name: Login to Docker - uses: docker/login-action@v2 + uses: docker/login-action@v3 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -112,14 +112,14 @@ jobs: - name: Docker meta id: docker_meta - uses: docker/metadata-action@v4 + uses: docker/metadata-action@v5 with: images: ${{ env.IMAGE_NAME }}-webhook flavor: | latest=false - name: Build and push sriov-network-operator-webhook - uses: docker/build-push-action@v4 + uses: docker/build-push-action@v6 with: context: . push: true diff --git a/.github/workflows/pr-ci-triggers.yml b/.github/workflows/pr-ci-triggers.yml index 2c38f6fec5..7b08903794 100644 --- a/.github/workflows/pr-ci-triggers.yml +++ b/.github/workflows/pr-ci-triggers.yml @@ -6,7 +6,7 @@ jobs: vendors-ci-triggers-list: runs-on: ubuntu-latest steps: - - uses: actions/github-script@v5 + - uses: actions/github-script@v7 env: MESSAGE: >- Thanks for your PR,\n diff --git a/.github/workflows/pr-labeler.yml b/.github/workflows/pr-labeler.yml index 27719aa160..9a1012b954 100644 --- a/.github/workflows/pr-labeler.yml +++ b/.github/workflows/pr-labeler.yml @@ -1,4 +1,4 @@ -name: "Pull Request Labeler" +name: Pull Request Labeler on: - pull_request_target @@ -6,6 +6,6 @@ jobs: triage: runs-on: ubuntu-latest steps: - - uses: actions/labeler@v3 + - uses: actions/labeler@v5 with: - repo-token: "${{ secrets.GITHUB_TOKEN }}" + repo-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/pr-update.yml b/.github/workflows/pr-update.yml index 09b6e1d98e..863479e710 100644 --- a/.github/workflows/pr-update.yml +++ b/.github/workflows/pr-update.yml @@ -1,12 +1,12 @@ -name: "Run Jobs on PR" +name: Run Jobs on PR on: pull_request jobs: execute: - name: "remove the lgtm label" + name: remove the lgtm label runs-on: ubuntu-latest steps: - uses: jpmcb/prow-github-actions@v1.1.2 with: - jobs: 'lgtm' - github-token: "${{ secrets.GITHUB_TOKEN }}" + jobs: lgtm + github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/prow.yml b/.github/workflows/prow.yml index 3f25bcab87..126561183e 100644 --- a/.github/workflows/prow.yml +++ b/.github/workflows/prow.yml @@ -1,4 +1,4 @@ -name: "Prow github actions" +name: Prow github actions on: issue_comment: types: [created] @@ -9,7 +9,8 @@ jobs: steps: - uses: jpmcb/prow-github-actions@v1.1.2 with: - prow-commands: '/assign + prow-commands: >- + /assign /unassign /approve /retitle @@ -24,5 +25,5 @@ jobs: /milestone /hold /cc - /uncc' - github-token: "${{ secrets.GITHUB_TOKEN }}" + /uncc + github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d59e52e470..087395dc3c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,12 +14,12 @@ jobs: steps: - name: Set up Go 1.22 - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: fmt run: IS_CONTAINER=yes make fmt @@ -33,12 +33,12 @@ jobs: steps: - name: Set up Go 1.22 - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: test pkg run: make test-pkg @@ -64,12 +64,12 @@ jobs: steps: - name: Set up Go 1.22 - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: check go modules are up to date run: make check-deps @@ -79,7 +79,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out code - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: check if the config/ folder is up to date run: make check-manifests @@ -89,16 +89,15 @@ jobs: runs-on: ubuntu-latest steps: - name: Set up Go 1.22 - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: golangci-lint - uses: golangci/golangci-lint-action@v3 + uses: golangci/golangci-lint-action@v7 with: - # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.55.2 + version: v2.10.1 shellcheck: name: Shellcheck @@ -116,12 +115,12 @@ jobs: steps: - name: Set up Go 1.22 - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: test pkg run: make test-pkg @@ -145,6 +144,7 @@ jobs: path-to-lcov: lcov.out virtual-k8s-cluster: + if: ${{ vars.ENABLE_E2E == 'true' }} # requires self-hosted [sriov] runner name: k8s needs: [ build, @@ -156,17 +156,17 @@ jobs: TEST_REPORT_PATH: k8s-artifacts steps: - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Set up Go 1.22 - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: run test run: make test-e2e-conformance-virtual-k8s-cluster-ci - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: always() with: name: ${{ env.TEST_REPORT_PATH }} @@ -175,10 +175,11 @@ jobs: - name: remove virtual cluster if: always() run: | - kcli delete cluster $CLUSTER_NAME -y - kcli delete network $CLUSTER_NAME -y + kcli delete cluster "$CLUSTER_NAME" -y + kcli delete network "$CLUSTER_NAME" -y virtual-ocp: + if: ${{ vars.ENABLE_E2E == 'true' }} # requires self-hosted [ocp] runner name: ocp needs: [ build, @@ -190,17 +191,17 @@ jobs: TEST_REPORT_PATH: ocp-artifacts steps: - name: Check out code into the Go module directory - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Set up Go 1.22 - uses: actions/setup-go@v3 + uses: actions/setup-go@v5 with: go-version: 1.22.x - name: run test run: make test-e2e-conformance-virtual-ocp-cluster-ci - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: always() with: name: ${{ env.TEST_REPORT_PATH }} @@ -209,5 +210,5 @@ jobs: - name: remove virtual cluster if: always() run: | - kcli delete cluster $CLUSTER_NAME -y - kcli delete network $CLUSTER_NAME -y + kcli delete cluster "$CLUSTER_NAME" -y + kcli delete network "$CLUSTER_NAME" -y diff --git a/.gitignore b/.gitignore index 6761d7ea12..1419acd200 100644 --- a/.gitignore +++ b/.gitignore @@ -77,7 +77,6 @@ Session.vim # auto-generated tag files tags ### VisualStudioCode ### -.vscode/* .history # End of https://www.gitignore.io/api/go,vim,emacs,visualstudiocode #IDE (GoLand) specific diff --git a/.golangci.yml b/.golangci.yml index 253c4ff23e..c64484eec3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,123 +1,227 @@ -# Tested with golangci-lint ver. 1.37 -run: - timeout: 10m - skip-dirs: - - vendor/ - - .github/ - - deployment/ - - doc/ - - bindata/ - - pkg/client -linters-settings: - depguard: - rules: - main: - deny: - - pkg: github.com/sirupsen/logrus - desc: "logging is allowed only by logutils.Log" - dupl: - threshold: 100 - funlen: - lines: 100 - statements: 50 - goconst: - min-len: 2 - min-occurrences: 2 - ignore-tests: true - gocritic: - enabled-tags: - - diagnostic - - experimental - - opinionated - - performance - - style - disabled-checks: - - dupImport # https://github.com/go-critic/go-critic/issues/845 - - ifElseChain - - octalLiteral - - whyNoLint - - wrapperFunc - - unnamedResult - settings: - hugeParam: - sizeThreshold: 512 - rangeValCopy: - sizeThreshold: 512 - gocyclo: - min-complexity: 15 - goimports: - local-prefixes: github.com/k8snetworkplumbingwg/sriov-network-operator - gomnd: - settings: - mnd: - # don't include the "operation" and "assign" - checks: argument,case,condition,return - lll: - line-length: 140 - misspell: - locale: US - prealloc: - # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them. - # True by default. - simple: true - range-loops: true # Report preallocation suggestions on range loops, true by default - for-loops: false # Report preallocation suggestions on for loops, false by default - stylecheck: - dot-import-whitelist: - - github.com/onsi/ginkgo/v2 - - github.com/onsi/gomega - - github.com/onsi/gomega/gstruct +version: "2" linters: - # please, do not use `enable-all`: it's deprecated and will be removed soon. - # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint - disable-all: true + default: none + enable: + # --- Bugs & Correctness --- + - bodyclose # checks whether HTTP response body is closed + - exhaustive # checks exhaustiveness of enum switch statements + - staticcheck # comprehensive static analysis (replaces stylecheck, gosimple) + # - durationcheck # checks for two durations multiplied together + # - nilerr # finds code that returns nil even after checking err != nil + # - noctx # detects HTTP requests without context.Context + # - rowserrcheck # checks whether sql.Rows.Err is checked + # - sqlclosecheck # checks that sql.Rows, sql.Stmt are closed + # - govet # reports suspicious constructs (go vet) + # - contextcheck # checks for non-inherited context usage + # - errcheck # checks for unchecked errors — noisy, enable incrementally + # - nilnesserr # reports err != nil checks that return a different nil error + # - nilnil # checks no simultaneous return of nil error and invalid value + # - spancheck # checks for mistakes with OpenTelemetry spans + # - musttag # enforces field tags in (un)marshaled structs + + # --- Error Handling --- + # - errname # checks that sentinel errors are Err-prefixed, error types Error-suffixed + # - errorlint # finds issues with error wrapping (Go 1.13+) + # - err113 # checks error handling expressions (errors.New in returns, etc.) + # - wrapcheck # checks errors from external packages are wrapped + + # --- Performance --- + - ineffassign # detects unused assignments to existing variables + - unconvert # removes unnecessary type conversions + # - wastedassign # finds wasted assignment statements + # - prealloc # finds slice declarations that could be pre-allocated + # - perfsprint # checks fmt.Sprintf replaceable with faster alternatives + + # --- Style & Consistency --- + - dogsled # checks assignments with too many blank identifiers + - goconst # finds repeated strings replaceable by constants + - goprintffuncname # checks printf-like functions are named with f suffix + - misspell # finds commonly misspelled English words + - unused # checks for unused constants, variables, functions, types + - whitespace # checks for unnecessary newlines at start/end of blocks + - depguard # checks package imports against allow/deny lists + # - dupword # checks for duplicate words in source code + # - godot # checks if comments end in a period + # - nakedret # checks functions with naked returns aren't too long + # - nlreturn # checks for newline before return/branch statements + # - revive # configurable drop-in replacement for golint + # - predeclared # finds code that shadows Go's predeclared identifiers + # - usestdlibvars # detects opportunity to use stdlib variables/constants + # - nonamedreturns # reports all named returns + + # --- Complexity --- + # - gocyclo # computes and checks cyclomatic complexity + # - gocognit # computes and checks cognitive complexity + # - funlen # checks for long functions + # - cyclop # checks function and package cyclomatic complexity + # - maintidx # measures maintainability index + # - nestif # reports deeply nested if statements + + # --- Code Patterns --- + - nosprintfhostport # checks for misuse of Sprintf to construct host:port URLs + # - ginkgolinter # enforces ginkgo/gomega standards + # - gocritic # diagnostics for bugs, performance, style issues + # - forcetypeassert # finds forced type assertions without ok check + # - exhaustruct # checks all struct fields are initialized + # - containedctx # detects struct containing context.Context + # - reassign # checks package variables are not reassigned + # - ireturn # accept interfaces, return concrete types + # - varnamelen # checks variable name length matches scope + + # --- Security --- + # - gosec # inspects source code for security problems + + # --- Modules & Imports --- + # - gomoddirectives # manages replace/retract/exclude in go.mod + # - gomodguard # allow/blocklist for direct Go module dependencies + # - importas # enforces consistent import aliases + + # --- SQL --- + # - unqueryvet # detects SELECT * in SQL queries + + # --- Logging --- + # - loggercheck # checks key-value pairs for common loggers (zap, slog, logr) + # - sloglint # ensures consistent log/slog style + # - zerologlint # detects wrong zerolog usage + + # --- Testing --- + # - paralleltest # detects missing t.Parallel() in tests + # - tparallel # detects inappropriate t.Parallel() usage + # - thelper # detects test helpers without t.Helper() + # - testpackage # enforces use of separate _test package + # - testifylint # checks usage of testify + # - usetesting # reports functions replaceable with testing package + + # --- Modernization --- + # - modernize # suggests simplifications using modern Go features + # - exptostd # detects x/exp functions replaceable by stdlib + # - intrange # finds for loops that could use integer range + # - copyloopvar # detects unnecessary loop variable copies (Go 1.22+) + # - mirror # reports wrong mirror patterns of bytes/strings usage + + disable: + # --- Not applicable to this project --- + - arangolint # arangodb client best practices — not used + - protogetter # proto message field getters — not used + - tagliatelle # struct tag naming conventions — conflicts with k8s json tags + - gosmopolitan # i18n anti-patterns — not applicable + - zerologlint # zerolog usage — not used + - testifylint # testify usage — project uses ginkgo/gomega + - inamedparam # unnamed interface params — too noisy for k8s-style interfaces + + # --- Too opinionated / noisy --- + - exhaustruct # require all struct fields initialized — impractical with k8s types + - varnamelen # variable name length — too strict + - wsl_v5 # whitespace formatting — too opinionated + - gochecknoglobals # no globals — impractical for operator pattern + - gochecknoinits # no init functions — impractical for scheme registration + - nlreturn # newline before return — too noisy + - noinlineerr # no inline error handling — too restrictive + - decorder # declaration order — too strict + - grouper # expression group analysis — too opinionated + - funcorder # function order — too strict + - embeddedstructfieldcheck # embedded struct field ordering — too opinionated + - iotamixing # iota in mixed const blocks — too strict + - godox # flags TODO/FIXME — useful but too noisy during development + + # --- Deprecated --- + - wsl # deprecated in favor of wsl_v5 + + settings: + depguard: + rules: + main: + deny: + - pkg: github.com/sirupsen/logrus + desc: logging is allowed only by logutils.Log + dupl: + threshold: 100 + funlen: + lines: 100 + statements: 50 + goconst: + min-len: 2 + min-occurrences: 2 + gocritic: + disabled-checks: + - dupImport + - ifElseChain + - octalLiteral + - whyNoLint + - wrapperFunc + - unnamedResult + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + settings: + hugeParam: + sizeThreshold: 512 + rangeValCopy: + sizeThreshold: 512 + gocyclo: + min-complexity: 15 + lll: + line-length: 140 + misspell: + locale: US + mnd: + checks: + - argument + - case + - condition + - return + prealloc: + simple: true + range-loops: true + for-loops: false + staticcheck: + dot-import-whitelist: + - github.com/onsi/ginkgo/v2 + - github.com/onsi/gomega + - github.com/onsi/gomega/gstruct + + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - dupl + - goconst + - gosec + - lll + - mnd + - staticcheck + path: _test\.go + - linters: + - goconst + path: test/conformance/tests + - linters: + - goconst + path: (.+)_test\.go + paths: + - third_party$ + - builtin$ + - examples$ + +formatters: enable: - - bodyclose - - depguard - - dogsled - # TODO fix issues- dupl - # TODO fix issues- errcheck - - exportloopref - - exhaustive - # TODO fix issues- funlen - #- gochecknoinits - - goconst - # TODO fix issues- gocritic - # TODO fix issues- gocyclo - gofmt - goimports - # TODO fix issues- gomnd - - goprintffuncname - # TODO fix issues- gosec - - gosimple - #- govet - - ineffassign - # TODO fix issues- lll - - misspell - # TODO fix issues- nakedret - # TODO fix issues- prealloc - #- scopelint - # TODO fix issues- staticcheck - - stylecheck - - typecheck - - unconvert - # TODO fix issues- unparam - - unused - - whitespace - -issues: - # Excluding configuration per-path, per-linter, per-text and per-source - exclude-rules: - - path: _test\.go - linters: - - gomnd - - gosec - - dupl - - lll - - stylecheck - - goconst - - path: test/conformance/tests - linters: - - goconst - + settings: + goimports: + local-prefixes: + - github.com/k8snetworkplumbingwg/sriov-network-operator + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/.trunk/.gitignore b/.trunk/.gitignore new file mode 100644 index 0000000000..15966d087e --- /dev/null +++ b/.trunk/.gitignore @@ -0,0 +1,9 @@ +*out +*logs +*actions +*notifications +*tools +plugins +user_trunk.yaml +user.yaml +tmp diff --git a/.trunk/configs/.hadolint.yaml b/.trunk/configs/.hadolint.yaml new file mode 100644 index 0000000000..98bf0cd2ee --- /dev/null +++ b/.trunk/configs/.hadolint.yaml @@ -0,0 +1,4 @@ +# Following source doesn't work in most setups +ignored: + - SC1090 + - SC1091 diff --git a/.trunk/configs/.markdownlint.yaml b/.trunk/configs/.markdownlint.yaml new file mode 100644 index 0000000000..b40ee9d7a7 --- /dev/null +++ b/.trunk/configs/.markdownlint.yaml @@ -0,0 +1,2 @@ +# Prettier friendly markdownlint config (all formatting rules disabled) +extends: markdownlint/style/prettier diff --git a/.trunk/configs/.shellcheckrc b/.trunk/configs/.shellcheckrc new file mode 100644 index 0000000000..8c7b1ada8a --- /dev/null +++ b/.trunk/configs/.shellcheckrc @@ -0,0 +1,7 @@ +enable=all +source-path=SCRIPTDIR +disable=SC2154 + +# If you're having issues with shellcheck following source, disable the errors via: +# disable=SC1090 +# disable=SC1091 diff --git a/.trunk/configs/.yamllint.yaml b/.trunk/configs/.yamllint.yaml new file mode 100644 index 0000000000..184e251f8d --- /dev/null +++ b/.trunk/configs/.yamllint.yaml @@ -0,0 +1,7 @@ +rules: + quoted-strings: + required: only-when-needed + extra-allowed: ["{|}"] + key-duplicates: {} + octal-values: + forbid-implicit-octal: true diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml new file mode 100644 index 0000000000..783dbeb9ea --- /dev/null +++ b/.trunk/trunk.yaml @@ -0,0 +1,53 @@ +# This file controls the behavior of Trunk: https://docs.trunk.io/cli +# To learn more about the format of this file, see https://docs.trunk.io/reference/trunk-yaml +version: 0.1 +cli: + version: 1.25.0 +# Trunk provides extensibility via plugins. (https://docs.trunk.io/plugins) +plugins: + sources: + - id: trunk + ref: v1.7.5 + uri: https://github.com/trunk-io/plugins +# Many linters and tools depend on runtimes - configure them here. (https://docs.trunk.io/runtimes) +runtimes: + enabled: + - go@1.22.12 + - node@22.16.0 + - python@3.10.8 +# This is the section where you manage your linters. (https://docs.trunk.io/check/configuration) +lint: + ignore: + - linters: [ ALL ] + paths: + - .trunk/** + - .claude/** + - CLAUDE.md + definitions: + - name: golangci-lint2 + environment: + - name: GOOS + value: linux + disabled: + - osv-scanner + - checkov + enabled: + - actionlint@1.7.11 + - git-diff-check + - gofmt@1.22.12 + - golangci-lint2@2.10.1 + - hadolint@2.14.0 + - markdownlint@0.48.0 + - oxipng@10.1.0 + - biome@2.4.5 + - shellcheck@0.11.0 + - shfmt@3.6.0 + - trivy@0.69.3 + - trufflehog@3.93.7 + - yamllint@1.38.0 +actions: + enabled: + - trunk-announce + - trunk-check-pre-push + - trunk-fmt-pre-commit + - trunk-upgrade-available diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000000..c7d046d3e8 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "go.toolsEnvVars": { + "GOOS": "linux" + } +} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 979ea417c8..3e915b3d11 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,16 +5,15 @@ * [Coding Style](#coding-style) * [Format of the patch](#format-of-the-patch) * [Contributing Code](#contributing-code) -* [Tools](#Tools) ## Meetings + Join us for project discussions at _K8s Network & Resource management_ meetings. The meetings take place on a weekly basis on Monday and Tuesday in alternating weeks: * Time: 15:00 - 16:00 GMT / 10:00-11:00 ET / 07:00-08:00 PST every other Monday * Time: 14:00 - 15:00 GMT / 09:00-10:00 ET / 06:00-07:00 PST on every other Tuesday - * [Meeting notes and agenda](https://docs.google.com/document/d/1sJQMHbxZdeYJPgAWK1aSt6yzZ4K_8es7woVIrwinVwI/edit?usp=sharing) * [Zoom channel](https://us02web.zoom.us/j/88973596231?pwd=UHZtUEtNSVBSYnRLL3A3cHMzdmlvZz09) @@ -26,13 +25,16 @@ contact points and other resources to make it easier to get your contribution ac ## Coding Style -Please follows the standard formatting recommendations and language idioms set out in [Effective Go](https://golang.org/doc/effective_go.html) and in the [Go Code Review Comments wiki](https://github.com/golang/go/wiki/CodeReviewComments). +Please follows the standard formatting recommendations and language +idioms set out in [Effective Go](https://golang.org/doc/effective_go.html) +and in the +[Go Code Review Comments wiki](https://github.com/golang/go/wiki/CodeReviewComments). ## Format of the patch Each patch is expected to comply with the following format: -``` +```text Change summary More detailed explanation of your changes: Why and how. @@ -45,49 +47,69 @@ for some more good advices. For example: -``` +```text Fix poorly named identifiers - + One identifier, fnname, in func.go was poorly named. It has been renamed to fnName. Another identifier retval was not needed and has been removed entirely. Fixes #1 -``` +``` ## Contributing Code -We always encourage the contribution for the community project. We like to collaborate with various stake holder on this project. We ask developer to keep following guidelines in mind before the contribution. - -* Make sure to create an [Issue](https://github.com/k8snetworkplumbingwg/sriov-network-operator/issues) for bug fix or the feature request. -Issues are discussed on a regular basis at _K8s Network & Resource management_ meetings. -* **For bugs**: For the bug fixes, please follow the issue template format while creating a issue. If you have already found a fix, feel free to submit a Pull Request referencing the Issue you created. Include the `Fixes #` syntax to link it to the issue you're addressing. -* **For feature requests**, For the feature requests, please follow the issue template format while creating a feature requests. We want to improve upon SRIOV Network Operator incrementally which means small changes or features at a time. +We always encourage the contribution for the community project. We like +to collaborate with various stake holder on this project. We ask +developer to keep following guidelines in mind before the contribution. + +* Make sure to create an + [Issue](https://github.com/k8snetworkplumbingwg/sriov-network-operator/issues) + for bug fix or the feature request. + Issues are discussed on a regular basis at + _K8s Network & Resource management_ meetings. +* **For bugs**: For the bug fixes, please follow the issue template + format while creating a issue. If you have already found a fix, feel + free to submit a Pull Request referencing the Issue you created. + Include the `Fixes #` syntax to link it to the issue you're + addressing. +* **For feature requests**, For the feature requests, please follow the + issue template format while creating a feature requests. We want to + improve upon SRIOV Network Operator incrementally which means small + changes or features at a time. * Please make sure each PR are compiling or passed by Travis. - * In order to ensure your PR can be reviewed in a timely manner, please keep PRs small + * In order to ensure your PR can be reviewed in a timely manner, + please keep PRs small Once you're ready to contribute code back to this repo, start with these steps: + * Fork the appropriate sub-projects that are affected by your change. * Clone the fork to your machine: -``` -$ git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git +```bash +git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git ``` * Create a topic branch with prefix `dev/` for your change and checkout that branch: +```bash +git checkout -b dev/some-topic-branch ``` -$ git checkout -b dev/some-topic-branch -``` + * Make your changes to the code and add tests to cover contributed code. * Run `make all` to validate it builds and will not break current functionality. * Commit your changes and push them to your fork. * Open a pull request for the appropriate project. -* Contributors will review your pull request, suggest changes, run tests and eventually merge or close the request. +* Contributors will review your pull request, suggest changes, run + tests and eventually merge or close the request. > We encourage contributor to test SRIOV Network Operator with various NICs to check the compatibility. -> + ## Contact Us -- General channel on [NPWG](https://npwg-team.slack.com/) Slack. Request an invite to NPWG slack [here](https://intel-corp.herokuapp.com/). -- Feel free to post GitHub issues and PRs for review -- Attend either K8s Network & Resource mangement or Additional K8s Network & Resource management meetings + +* General channel on [NPWG](https://npwg-team.slack.com/) Slack. + Request an invite to NPWG slack + [via this form](https://intel-corp.herokuapp.com/). +* Feel free to post GitHub issues and PRs for review +* Attend either K8s Network & Resource mangement or Additional K8s + Network & Resource management meetings diff --git a/Makefile b/Makefile index e70b591ac8..2eca0af59c 100644 --- a/Makefile +++ b/Makefile @@ -28,6 +28,7 @@ MAIN_PKG=cmd/manager/main.go export NAMESPACE?=openshift-sriov-network-operator export WATCH_NAMESPACE?=openshift-sriov-network-operator export HOME?=$(PWD) +ENVTEST_K8S_VERSION?=1.29.0 export GOPATH?=$(shell go env GOPATH) export GO111MODULE=on PKGS=$(shell go list ./... | grep -v -E '/vendor/|/test|/examples') @@ -53,7 +54,7 @@ GOLANGCI_LINT = $(BIN_DIR)/golangci-lint # golangci-lint version should be updated periodically # we keep it fixed to avoid it from unexpectedly failing on the project # in case of a version bump -GOLANGCI_LINT_VER = v1.55.2 +GOLANGCI_LINT_VER = v2.10.1 .PHONY: all build clean gendeepcopy test test-e2e test-e2e-k8s run image fmt sync-manifests test-e2e-conformance manifests update-codegen @@ -79,7 +80,7 @@ image: ; $(info Building images...) # Run tests test: generate lint manifests envtest - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test -coverprofile cover.out -v ${TESTPKGS} + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test -coverprofile cover.out -count=1 -v ${TESTPKGS} # Build manager binary manager: generate _build-manager @@ -157,7 +158,7 @@ kustomize: ## Download kustomize locally if necessary. ENVTEST = $(BIN_DIR)/setup-envtest envtest: ## Download envtest-setup locally if necessary. - $(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@release-0.16) + $(call go-install-tool,$(ENVTEST),sigs.k8s.io/controller-runtime/tools/setup-envtest@latest) GOMOCK = $(shell pwd)/bin/mockgen gomock: @@ -229,7 +230,7 @@ test-bindata-scripts: fakechroot fakechroot ./test/scripts/kargs_test.sh test-%: generate manifests envtest - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./$*/... -coverprofile cover-$*-$(CLUSTER_TYPE).out -coverpkg ./... -v + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./$*/... -coverprofile cover-$*-$(CLUSTER_TYPE).out -coverpkg ./... -count=1 -v GOCOVMERGE = $(BIN_DIR)/gocovmerge gocovmerge: ## Download gocovmerge locally if necessary. @@ -263,11 +264,15 @@ check-deps: deps-update exit 1; fi $(GOLANGCI_LINT): ; $(info installing golangci-lint...) - $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VER)) + $(call go-install-tool,$(GOLANGCI_LINT),github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VER)) .PHONY: lint lint: | $(GOLANGCI_LINT) ; $(info running golangci-lint...) @ ## Run golangci-lint - $(GOLANGCI_LINT) run --timeout=10m + GOOS=linux $(GOLANGCI_LINT) run + +.PHONY: lint-fix +lint-fix: | $(GOLANGCI_LINT) ; $(info running golangci-lint --fix...) @ ## Run golangci-lint with auto-fix + GOOS=linux $(GOLANGCI_LINT) run --fix $(BIN_DIR): @mkdir -p $(BIN_DIR) diff --git a/README.md b/README.md index 9759b0574b..e3b3d98391 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,16 @@ # sriov-network-operator -The Sriov Network Operator is designed to help the user to provision and configure SR-IOV CNI plugin and Device plugin in the Openshift cluster. +The Sriov Network Operator is designed to help the user to provision and +configure SR-IOV CNI plugin and Device plugin in the Openshift cluster. ## Motivation -SR-IOV network is an optional feature of an Openshift cluster. To make it work, it requires different components to be provisioned and configured accordingly. It makes sense to have one operator to coordinate those relevant components in one place, instead of having them managed by different operators. And also, to hide the complexity, we should provide an elegant user interface to simplify the process of enabling SR-IOV. +SR-IOV network is an optional feature of an Openshift cluster. To make it +work, it requires different components to be provisioned and configured +accordingly. It makes sense to have one operator to coordinate those relevant +components in one place, instead of having them managed by different operators. +And also, to hide the complexity, we should provide an elegant user interface +to simplify the process of enabling SR-IOV. ## Features @@ -36,9 +42,15 @@ The SR-IOV network operator introduces following new CRDs: ### SriovNetwork -A custom resource of SriovNetwork could represent the a layer-2 broadcast domain where some SR-IOV devices are attach to. It is primarily used to generate a NetworkAttachmentDefinition CR with an SR-IOV CNI plugin configuration. +A custom resource of SriovNetwork could represent the a layer-2 broadcast +domain where some SR-IOV devices are attach to. It is primarily used to +generate a NetworkAttachmentDefinition CR with an SR-IOV CNI plugin +configuration. -This SriovNetwork CR also contains the ‘resourceName’ which is aligned with the ‘resourceName’ of SR-IOV device plugin. One SriovNetwork obj maps to one ‘resoureName’, but one ‘resourceName’ can be shared by different SriovNetwork CRs. +This SriovNetwork CR also contains the ‘resourceName’ which is aligned with +the ‘resourceName’ of SR-IOV device plugin. One SriovNetwork obj maps to one +‘resoureName’, but one ‘resourceName’ can be shared by different SriovNetwork +CRs. This CR should be managed by cluster admin. Here is an example: @@ -66,9 +78,13 @@ spec: #### Chaining CNI metaplugins -It is possible to add additional capabilities to the device configured via the SR-IOV configuring optional metaplugins. +It is possible to add additional capabilities to the device configured via +the SR-IOV configuring optional metaplugins. -In order to do this, the `metaPlugins` field must contain the array of one or more additional configurations used to build a [network configuration list](https://github.com/containernetworking/cni/blob/master/SPEC.md#network-configuration-lists), as per the following example: +In order to do this, the `metaPlugins` field must contain the array of one or +more additional configurations used to build a +[network configuration list](https://github.com/containernetworking/cni/blob/master/SPEC.md#network-configuration-lists), +as per the following example: ```yaml apiVersion: sriovnetwork.openshift.io/v1 @@ -105,13 +121,20 @@ spec: ### OVSNetwork -A custom resource of OVSNetwork could represent the a layer-2 broadcast domain attached to Open vSwitch that works in HW-offloading mode. -It is primarily used to generate a NetworkAttachmentDefinition CR with an OVS CNI plugin configuration. +A custom resource of OVSNetwork could represent the a layer-2 broadcast +domain attached to Open vSwitch that works in HW-offloading mode. It is +primarily used to generate a NetworkAttachmentDefinition CR with an OVS CNI +plugin configuration. -The OVSNetwork CR also contains the `resourceName` which is aligned with the `resourceName` of SR-IOV device plugin. One OVSNetwork obj maps to one `resourceName`, but one `resourceName` can be shared by different OVSNetwork CRs. +The OVSNetwork CR also contains the `resourceName` which is aligned with the +`resourceName` of SR-IOV device plugin. One OVSNetwork obj maps to one +`resourceName`, but one `resourceName` can be shared by different OVSNetwork +CRs. -It is expected that `resourceName` contains name of the resource pool which holds Virtual Functions of a NIC in the switchdev mode. -A Physical function of the NIC should be attached to an OVS bridge before any workload which uses OVSNetwork starts. +It is expected that `resourceName` contains name of the resource pool which +holds Virtual Functions of a NIC in the switchdev mode. A Physical function +of the NIC should be attached to an OVS bridge before any workload which uses +OVSNetwork starts. Example: @@ -141,12 +164,19 @@ spec: ### SriovNetworkNodeState -The custom resource to represent the SR-IOV interface states of each host, which should only be managed by the operator itself. +The custom resource to represent the SR-IOV interface states of each host, +which should only be managed by the operator itself. -- The ‘spec’ of this CR represents the desired configuration which should be applied to the interfaces and SR-IOV device plugin. -- The ‘status’ contains current states of those PFs (baremetal only), and the states of the VFs. It helps user to discover SR-IOV network hardware on node, or attached VFs in the case of a virtual deployment. +- The ‘spec’ of this CR represents the desired configuration which should be + applied to the interfaces and SR-IOV device plugin. +- The ‘status’ contains current states of those PFs (baremetal only), and the + states of the VFs. It helps user to discover SR-IOV network hardware on + node, or attached VFs in the case of a virtual deployment. -The spec is rendered by sriov-policy-controller, and consumed by sriov-config-daemon. Sriov-config-daemon is responsible for updating the ‘status’ field to reflect the latest status, this information can be used as input to create SriovNetworkNodePolicy CR. +The spec is rendered by sriov-policy-controller, and consumed by +sriov-config-daemon. Sriov-config-daemon is responsible for updating the +‘status’ field to reflect the latest status, this information can be used as +input to create SriovNetworkNodePolicy CR. An example of SriovNetworkNodeState CR: @@ -196,13 +226,20 @@ status: vendor: "8086" ``` -From this example, in status field, the user can find out there are 2 SRIOV capable NICs on node 'work-node-1'; in spec field, user can learn what the expected configure is generated from the combination of SriovNetworkNodePolicy CRs. In the virtual deployment case, a single VF will be associated with each device. +From this example, in status field, the user can find out there are 2 SRIOV +capable NICs on node 'work-node-1'; in spec field, user can learn what the +expected configure is generated from the combination of SriovNetworkNodePolicy +CRs. In the virtual deployment case, a single VF will be associated with each +device. ### SriovNetworkNodePolicy -This CRD is the key of SR-IOV network operator. This custom resource should be managed by cluster admin, to instruct the operator to: +This CRD is the key of SR-IOV network operator. This custom resource should +be managed by cluster admin, to instruct the operator to: -1. Render the spec of SriovNetworkNodeState CR for selected node, to configure the SR-IOV interfaces. In virtual deployment, the VF interface is read-only. +1. Render the spec of SriovNetworkNodeState CR for selected node, to configure + the SR-IOV interfaces. In virtual deployment, the VF interface is + read-only. 2. Deploy SR-IOV CNI plugin and device plugin on selected node. 3. Generate the configuration of SR-IOV device plugin. @@ -229,60 +266,75 @@ spec: resourceName: intelnics ``` -In this example, user selected the nic from vendor '8086' which is intel, device module is '1583' which is XL710 for 40GbE, on nodes labeled with 'network-sriov.capable' equals 'true'. Then for those PFs, create 4 VFs each, set mtu to 1500 and the load the vfio-pci driver to those virtual functions. +In this example, user selected the nic from vendor '8086' which is intel, +device module is '1583' which is XL710 for 40GbE, on nodes labeled with +'network-sriov.capable' equals 'true'. Then for those PFs, create 4 VFs each, +set mtu to 1500 and the load the vfio-pci driver to those virtual functions. -In a virtual deployment: -- The mtu of the PF is set by the underlying virtualization platform and cannot be changed by the sriov-network-operator. +In a virtual deployment: + +- The mtu of the PF is set by the underlying virtualization platform and + cannot be changed by the sriov-network-operator. - The numVfs parameter has no effect as there is always 1 VF -- The deviceType field depends upon whether the underlying device/driver is [native-bifurcating or non-bifurcating](https://doc.dpdk.org/guides/howto/flow_bifurcation.html) For example, the supported Mellanox devices support native-bifurcating drivers and therefore deviceType should be netdevice (default). The support Intel devices are non-bifurcating and should be set to vfio-pci. +- The deviceType field depends upon whether the underlying device/driver is + [native-bifurcating or non-bifurcating](https://doc.dpdk.org/guides/howto/flow_bifurcation.html). + For example, the supported Mellanox devices support native-bifurcating + drivers and therefore deviceType should be netdevice (default). The support + Intel devices are non-bifurcating and should be set to vfio-pci. #### Multiple policies -When multiple SriovNetworkNodeConfigPolicy CRs are present, the `priority` field -(0 is the highest priority) is used to resolve any conflicts. Conflicts occur -only when same PF is referenced by multiple policies. The final desired +When multiple SriovNetworkNodeConfigPolicy CRs are present, the `priority` +field (0 is the highest priority) is used to resolve any conflicts. Conflicts +occur only when same PF is referenced by multiple policies. The final desired configuration is saved in `SriovNetworkNodeState.spec.interfaces`. -Policies processing order is based on priority (lowest first), followed by `name` -field (starting from `a`). Policies with same **priority** or **non-overlapping -VF groups** (when #-notation is used in pfName field) are merged, otherwise only -the highest priority policy is applied. In case of same-priority policies and -overlapping VF groups, only the last processed policy is applied. +Policies processing order is based on priority (lowest first), followed by +`name` field (starting from `a`). Policies with same **priority** or +**non-overlapping VF groups** (when #-notation is used in pfName field) are +merged, otherwise only the highest priority policy is applied. In case of +same-priority policies and overlapping VF groups, only the last processed +policy is applied. -When using #-notation to define VF group, no actions are taken on virtual functions that -are not mentioned in any policy (e.g. if a policy defines a `vfio-pci` device group for a device, when -it is deleted the VF are not reset to the default driver). +When using #-notation to define VF group, no actions are taken on virtual +functions that are not mentioned in any policy (e.g. if a policy defines a +`vfio-pci` device group for a device, when it is deleted the VF are not reset +to the default driver). #### Externally Manage virtual functions -When `ExternallyManage` is request on a policy the operator will only skip the virtual function creation. -The operator will only bind the virtual functions to the requested driver and expose them via the device plugin. -Another difference when this field is requested in the policy is that when this policy is removed the operator -will not remove the virtual functions from the policy. +When `ExternallyManage` is request on a policy the operator will only skip the +virtual function creation. The operator will only bind the virtual functions to +the requested driver and expose them via the device plugin. Another difference +when this field is requested in the policy is that when this policy is removed +the operator will not remove the virtual functions from the policy. -*Note:* This means the user must create the virtual functions before they apply the policy or the webhook will reject -the policy creation. +*Note:* This means the user must create the virtual functions before they apply +the policy or the webhook will reject the policy creation. -It's possible to use something like nmstate kubernetes-nmstate or just a simple systemd file to create -the virtual functions on boot. +It's possible to use something like nmstate kubernetes-nmstate or just a +simple systemd file to create the virtual functions on boot. -This feature was created to support deployments where the user want to use some of the virtual funtions for the host -communication like storage network or out of band managment and the virtual functions must exist on boot and not only +This feature was created to support deployments where the user want to use some +of the virtual funtions for the host communication like storage network or out +of band managment and the virtual functions must exist on boot and not only after the operator and config-daemon are running. #### Disabling SR-IOV Config Daemon plugins -It is possible to disable SR-IOV network operator config daemon plugins in case their operation -is not needed or un-desirable. +It is possible to disable SR-IOV network operator config daemon plugins in +case their operation is not needed or un-desirable. -As an example, some plugins perform vendor specific firmware configuration -to enable SR-IOV (e.g `mellanox` plugin). certain deployment environments may prefer to perform such configuration -once during node provisioning, while ensuring the configuration will be compatible with any sriov network node policy -defined for the particular environment. This will reduce or completely eliminate the need for reboot of nodes during SR-IOV -configurations by the operator. +As an example, some plugins perform vendor specific firmware configuration to +enable SR-IOV (e.g `mellanox` plugin). Certain deployment environments may +prefer to perform such configuration once during node provisioning, while +ensuring the configuration will be compatible with any sriov network node +policy defined for the particular environment. This will reduce or completely +eliminate the need for reboot of nodes during SR-IOV configurations by the +operator. -This can be done by setting SriovOperatorConfig `default` CR `spec.disablePlugins` with the list of desired plugins -to disable. +This can be done by setting SriovOperatorConfig `default` CR +`spec.disablePlugins` with the list of desired plugins to disable. **Example**: @@ -305,12 +357,16 @@ spec: It is possible to drain more than one node at a time using this operator. -The configuration is done via the SriovNetworkNodePool, selecting a number of nodes using the node selector and how many -nodes in parallel from the pool the operator can drain in parallel. maxUnavailable can be a number or a percentage. - -> **NOTE**: every node can only be part of one pool, if a node is selected by more than one pool, then it will not be drained +The configuration is done via the SriovNetworkNodePool, selecting a number of +nodes using the node selector and how many nodes in parallel from the pool the +operator can drain in parallel. maxUnavailable can be a number or a +percentage. -> **NOTE**: If a node is not part of any pool it will have a default configuration of maxUnavailable 1 +> **NOTE**: every node can only be part of one pool, if a node is selected by +> more than one pool, then it will not be drained +> +> **NOTE**: If a node is not part of any pool it will have a default +> configuration of maxUnavailable 1 **Example**: @@ -331,33 +387,54 @@ spec: Feature gates are used to enable or disable specific features in the operator. -> **NOTE**: As features mature and graduate to stable status, default settings may change, and feature gates might be removed in future releases. Keep this in mind when configuring feature gates and ensure your environment is compatible with any updates. +> **NOTE**: As features mature and graduate to stable status, default settings +> may change, and feature gates might be removed in future releases. Keep this +> in mind when configuring feature gates and ensure your environment is +> compatible with any updates. ### Available Feature Gates 1. **Parallel NIC Configuration** (`parallelNicConfig`) - - **Description:** Allows the configuration of NICs in parallel, which can potentially reduce the time required for network setup. - - **Default:** Disabled -2. **Resource Injector Match Condition** (`resourceInjectorMatchCondition`) - - **Description:** Switches the resource injector's webhook failure policy from "Ignore" to "Fail" by utilizing the `MatchConditions` feature introduced in Kubernetes 1.28. This ensures the webhook only targets pods with the `k8s.v1.cni.cncf.io/networks` annotation, improving reliability without affecting other pods. - - **Default:** Disabled +- **Description:** Allows the configuration of NICs in parallel, which can + potentially reduce the time required for network setup. +- **Default:** Disabled + +1. **Resource Injector Match Condition** (`resourceInjectorMatchCondition`) + +- **Description:** Switches the resource injector's webhook failure policy + from "Ignore" to "Fail" by utilizing the `MatchConditions` feature + introduced in Kubernetes 1.28. This ensures the webhook only targets pods + with the `k8s.v1.cni.cncf.io/networks` annotation, improving reliability + without affecting other pods. +- **Default:** Disabled + +1. **Metrics Exporter** (`metricsExporter`) + +- **Description:** Enables the metrics exporter on the same node where the + config-daemon is running. This helps in collecting and exporting metrics + related to SR-IOV network devices. +- **Default:** Disabled + +1. **Manage Software Bridges** (`manageSoftwareBridges`) -3. **Metrics Exporter** (`metricsExporter`) - - **Description:** Enables the metrics exporter on the same node where the config-daemon is running. This helps in collecting and exporting metrics related to SR-IOV network devices. - - **Default:** Disabled +- **Description:** Allows the operator to manage software bridges. This + feature gate is useful for environments where bridge management is required. +- **Default:** Disabled -4. **Manage Software Bridges** (`manageSoftwareBridges`) - - **Description:** Allows the operator to manage software bridges. This feature gate is useful for environments where bridge management is required. - - **Default:** Disabled +1. **Mellanox Firmware Reset** (`mellanoxFirmwareReset`) -5. **Mellanox Firmware Reset** (`mellanoxFirmwareReset`) - - **Description:** Enables the firmware reset via `mstfwreset` before a system reboot. This feature is specific to Mellanox network devices and is used to ensure that the firmware is properly reset during system maintenance. - - **Default:** Disabled +- **Description:** Enables the firmware reset via `mstfwreset` before a + system reboot. This feature is specific to Mellanox network devices and is + used to ensure that the firmware is properly reset during system + maintenance. +- **Default:** Disabled ### Enabling Feature Gates -To enable a feature gate, add it to your configuration file or command line with the desired state. For example, to enable the `resourceInjectorMatchCondition` feature gate, you would specify: +To enable a feature gate, add it to your configuration file or command line +with the desired state. For example, to enable the +`resourceInjectorMatchCondition` feature gate, you would specify: ```yaml apiVersion: sriovnetwork.openshift.io/v1 @@ -386,7 +463,8 @@ The controller is responsible for: The sriov-config-daemon is responsible for: -1. Discover the SRIOV NICs on each node, then sync the status of SriovNetworkNodeState CR. +1. Discover the SRIOV NICs on each node, then sync the status of + SriovNetworkNodeState CR. 2. Take the spec of SriovNetworkNodeState CR as input to configure those NICs. ## Workflow diff --git a/api/v1/helper_test.go b/api/v1/helper_test.go index 06215d067f..de2f09dcb7 100644 --- a/api/v1/helper_test.go +++ b/api/v1/helper_test.go @@ -142,6 +142,7 @@ func newVhostVdpaNodePolicy() *v1.SriovNetworkNodePolicy { } func TestRendering(t *testing.T) { + t.Parallel() testtable := []struct { tname string network v1.SriovNetwork @@ -203,6 +204,7 @@ func TestRendering(t *testing.T) { } func TestIBRendering(t *testing.T) { + t.Parallel() testtable := []struct { tname string network v1.SriovIBNetwork @@ -250,6 +252,7 @@ func TestIBRendering(t *testing.T) { } func TestOVSRendering(t *testing.T) { + t.Parallel() testtable := []struct { tname string network v1.OVSNetwork @@ -343,6 +346,7 @@ func TestOVSRendering(t *testing.T) { } func TestSriovNetworkNodePolicyApply(t *testing.T) { + t.Parallel() testtable := []struct { tname string currentState *v1.SriovNetworkNodeState @@ -804,6 +808,7 @@ func TestSriovNetworkNodePolicyApply(t *testing.T) { } func TestVirtioVdpaNodePolicyApply(t *testing.T) { + t.Parallel() testtable := []struct { tname string currentState *v1.SriovNetworkNodeState @@ -851,6 +856,7 @@ func TestVirtioVdpaNodePolicyApply(t *testing.T) { } func TestVhostVdpaNodePolicyApply(t *testing.T) { + t.Parallel() testtable := []struct { tname string currentState *v1.SriovNetworkNodeState @@ -898,6 +904,7 @@ func TestVhostVdpaNodePolicyApply(t *testing.T) { } func TestGetEswitchModeFromSpec(t *testing.T) { + t.Parallel() testtable := []struct { tname string spec *v1.Interface @@ -930,6 +937,7 @@ func TestGetEswitchModeFromSpec(t *testing.T) { } func TestGetEswitchModeFromStatus(t *testing.T) { + t.Parallel() testtable := []struct { tname string spec *v1.InterfaceExt @@ -962,6 +970,7 @@ func TestGetEswitchModeFromStatus(t *testing.T) { } func TestSriovNetworkPoolConfig_MaxUnavailable(t *testing.T) { + t.Parallel() testtable := []struct { tname string maxUn intstrutil.IntOrString @@ -1056,6 +1065,7 @@ func TestSriovNetworkPoolConfig_MaxUnavailable(t *testing.T) { } func TestNeedToUpdateSriov(t *testing.T) { + t.Parallel() type args struct { ifaceSpec *v1.Interface ifaceStatus *v1.InterfaceExt @@ -1124,6 +1134,7 @@ func TestNeedToUpdateSriov(t *testing.T) { } func TestSriovNetworkNodePolicyApplyBridgeConfig(t *testing.T) { + t.Parallel() testtable := []struct { tname string currentState *v1.SriovNetworkNodeState @@ -1498,6 +1509,7 @@ func TestSriovNetworkNodePolicyApplyBridgeConfig(t *testing.T) { } func TestGenerateBridgeName(t *testing.T) { + t.Parallel() result := v1.GenerateBridgeName(&v1.InterfaceExt{PciAddress: "0000:86:00.2"}) expected := "br-0000_86_00.2" if result != expected { @@ -1506,6 +1518,7 @@ func TestGenerateBridgeName(t *testing.T) { } func TestNeedToUpdateBridges(t *testing.T) { + t.Parallel() testtable := []struct { tname string specBridge *v1.Bridges diff --git a/ci/README.md b/ci/README.md index eb5c83edf9..9875d0a2cf 100644 --- a/ci/README.md +++ b/ci/README.md @@ -1,41 +1,68 @@ -## CI configurations and examples -This folder holds vendors CI configurations and examples. Configurations are used to control the vendors CI behaviors, and examples are used as a reference for other vendors to be able to setup a CI of their own. +# CI configurations and examples -### Admin list -The admin list contains the list of github users and organizations that have permission to trigger the vendors CI. Only trusted users who have merge permissions should be on the list. The vendors should be responsible for how the admin list on their CI is updated, but to keep everything organized the vendors CI should at least update their admin list in response to a PR comment with the following phrase `/update-admins`. +This folder holds vendors CI configurations and examples. Configurations +are used to control the vendors CI behaviors, and examples are used as a +reference for other vendors to be able to setup a CI of their own. -### CI Examples -The examples folder contains configuration examples for vendor CI. It can be used as a reference for vendors to setup their CI. For more information on an example refer to that folder README. +## Admin list + +The admin list contains the list of github users and organizations that +have permission to trigger the vendors CI. Only trusted users who have +merge permissions should be on the list. The vendors should be +responsible for how the admin list on their CI is updated, but to keep +everything organized the vendors CI should at least update their admin +list in response to a PR comment with the following phrase +`/update-admins`. + +## CI Examples + +The examples folder contains configuration examples for vendor CI. It +can be used as a reference for vendors to setup their CI. For more +information on an example refer to that folder README. + +## Vendors CI triggers convention -### Vendors CI triggers convention A vendor CI trigger phrases should follow the following convention: -``` +```text /test--- /skip--- ``` where: - 1. `test-type`: The type of test to conduct on the vendor's setups, for example: e2e. It can be replaced with `all` to test all tests types and vendors, currently the following values are supported: - * `e2e`: Runs the project's e2e make rule on the specific vendor setup. - * `all`: Runs all test types and all their tests for all vendors. - 2. `vendor`: The vendor that implemented the CI, currently the following values are supported: - * `nvidia`: Runs tests implemented by NVIDIA, the following test types are supported: e2e. - * `all`: Runs the specified test type and all its sub tests for all vendors. + 1. `test-type`: The type of test to conduct on the vendor's setups, for + example: e2e. It can be replaced with `all` to test all tests types + and vendors, currently the following values are supported: + +* `e2e`: Runs the project's e2e make rule on the specific vendor setup. +* `all`: Runs all test types and all their tests for all vendors. - 3. `sub-test`: In case there are many tests for the `test-type`, this field specify what sub-test to run. currently the following values are supported: - * `all`: Runs all subtest of the specified vendor. + 1. `vendor`: The vendor that implemented the CI, currently the + following values are supported: + +* `nvidia`: Runs tests implemented by NVIDIA, the following test types are supported: e2e. +* `all`: Runs the specified test type and all its sub tests for all vendors. + + 1. `sub-test`: In case there are many tests for the `test-type`, this + field specify what sub-test to run. currently the following values + are supported: + +* `all`: Runs all subtest of the specified vendor. + +Note that *all fields are required* except if `all` is added to the +phrase, in that case, there must not be any field after the `all`. This +means that the following phrases are supported when `all` is used: -Note that *all fields are required* except if `all` is added to the phrase, in that case, there must not be any field after the `all`. This means that the following phrases are supported when `all` is used: 1. `/test-all` 2. `/test--all` 3. `/test---all` But the following are not: + 1. `/test-all-` 2. `/test-all--` 3. `/test--all-` -The skip would report pass without running the test, and its triggers follow the above convention as well. - +The skip would report pass without running the test, and its triggers +follow the above convention as well. diff --git a/ci/examples/jenkins/README.md b/ci/examples/jenkins/README.md index c95b812e9d..5a95cd6ba1 100644 --- a/ci/examples/jenkins/README.md +++ b/ci/examples/jenkins/README.md @@ -1,5 +1,10 @@ -## Jenkins CI examples +# Jenkins CI examples + This folder holds examples for jenkins CI. -### sriov-network-operator-ci.yaml -This file holds an example jenkins-job-builder configuration that would be triggered on PRs by the admin list, and would simply run the `hack/run-e2e-test-kind.sh` script with `system-service` netns device switcher. +## sriov-network-operator-ci.yaml + +This file holds an example jenkins-job-builder configuration that would +be triggered on PRs by the admin list, and would simply run the +`hack/run-e2e-test-kind.sh` script with `system-service` netns device +switcher. diff --git a/controllers/drain_controller_test.go b/controllers/drain_controller_test.go index de3fe08843..e04921e9ec 100644 --- a/controllers/drain_controller_test.go +++ b/controllers/drain_controller_test.go @@ -135,32 +135,46 @@ var _ = Describe("Drain Controller", Ordered, func() { node2, nodeState2 := createNode(ctx, "node2") node3, nodeState3 := createNode(ctx, "node3") + nodes := []*corev1.Node{node1, node2} + nodeStates := []*sriovnetworkv1.SriovNetworkNodeState{nodeState1, nodeState2} + // Two nodes require to drain at the same time simulateDaemonSetAnnotation(node1, constants.DrainRequired) simulateDaemonSetAnnotation(node2, constants.DrainRequired) - // Only the first node drains - expectNodeStateAnnotation(nodeState1, constants.DrainComplete) - expectNodeStateAnnotation(nodeState2, constants.DrainIdle) + // Exactly one of the two nodes drains (order is non-deterministic) + expectNumberOfDrainingNodes(1, nodeState1, nodeState2, nodeState3) expectNodeStateAnnotation(nodeState3, constants.DrainIdle) - expectNodeIsNotSchedulable(node1) - expectNodeIsSchedulable(node2) expectNodeIsSchedulable(node3) - simulateDaemonSetAnnotation(node1, constants.DrainIdle) + // Find which node drained first and which is waiting + var firstNode, secondNode *corev1.Node + var firstState, secondState *sriovnetworkv1.SriovNetworkNodeState + for i, ns := range nodeStates { + if utils.ObjectHasAnnotation(ns, constants.NodeStateDrainAnnotationCurrent, constants.DrainComplete) { + firstNode, firstState = nodes[i], nodeStates[i] + secondNode, secondState = nodes[1-i], nodeStates[1-i] + break + } + } + Expect(firstNode).ToNot(BeNil(), "expected one node to have DrainComplete") - expectNodeStateAnnotation(nodeState1, constants.DrainIdle) - expectNodeIsSchedulable(node1) + expectNodeIsNotSchedulable(firstNode) + expectNodeStateAnnotation(secondState, constants.DrainIdle) + expectNodeIsSchedulable(secondNode) + + // Complete drain on first node + simulateDaemonSetAnnotation(firstNode, constants.DrainIdle) + + expectNodeStateAnnotation(firstState, constants.DrainIdle) + expectNodeIsSchedulable(firstNode) // Second node starts draining - expectNodeStateAnnotation(nodeState1, constants.DrainIdle) - expectNodeStateAnnotation(nodeState2, constants.DrainComplete) - expectNodeStateAnnotation(nodeState3, constants.DrainIdle) - expectNodeIsSchedulable(node1) - expectNodeIsNotSchedulable(node2) - expectNodeIsSchedulable(node3) + expectNodeStateAnnotation(secondState, constants.DrainComplete) + expectNodeIsNotSchedulable(secondNode) - simulateDaemonSetAnnotation(node2, constants.DrainIdle) + // Complete drain on second node + simulateDaemonSetAnnotation(secondNode, constants.DrainIdle) expectNodeStateAnnotation(nodeState1, constants.DrainIdle) expectNodeStateAnnotation(nodeState2, constants.DrainIdle) diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index f8811ed977..3b77a71d12 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -226,7 +226,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncDevicePluginConfigMap(ctx context } configData[node.Name] = string(config) - if data.ResourceList == nil || len(data.ResourceList) == 0 { + if len(data.ResourceList) == 0 { // if we don't have policies we should add the disabled label for the device plugin err = utils.LabelNode(ctx, node.Name, constants.SriovDevicePluginLabel, constants.SriovDevicePluginLabelDisabled, r.Client) if err != nil { diff --git a/controllers/sriovnetworkpoolconfig_controller.go b/controllers/sriovnetworkpoolconfig_controller.go index 43fd513c9a..16b59834b2 100644 --- a/controllers/sriovnetworkpoolconfig_controller.go +++ b/controllers/sriovnetworkpoolconfig_controller.go @@ -79,12 +79,12 @@ func (r *SriovNetworkPoolConfigReconciler) Reconcile(ctx context.Context, req ct } // examine DeletionTimestamp to determine if object is under deletion - if instance.ObjectMeta.DeletionTimestamp.IsZero() { + if instance.DeletionTimestamp.IsZero() { // The object is not being deleted, so if it does not have our finalizer, // then lets add the finalizer and update the object. This is equivalent // registering our finalizer. - if !sriovnetworkv1.StringInArray(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.ObjectMeta.Finalizers) { - instance.ObjectMeta.Finalizers = append(instance.ObjectMeta.Finalizers, sriovnetworkv1.POOLCONFIGFINALIZERNAME) + if !sriovnetworkv1.StringInArray(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.Finalizers) { + instance.Finalizers = append(instance.Finalizers, sriovnetworkv1.POOLCONFIGFINALIZERNAME) if err := r.Update(ctx, instance); err != nil { return reconcile.Result{}, err } @@ -100,7 +100,7 @@ func (r *SriovNetworkPoolConfigReconciler) Reconcile(ctx context.Context, req ct } } else { // The object is being deleted - if sriovnetworkv1.StringInArray(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.ObjectMeta.Finalizers) { + if sriovnetworkv1.StringInArray(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.Finalizers) { // our finalizer is present, so lets handle any external dependency logger.Info("delete SriovNetworkPoolConfig CR", "Namespace", instance.Namespace, "Name", instance.Name) if vars.ClusterType == constants.ClusterTypeOpenshift && !isHypershift { @@ -112,7 +112,7 @@ func (r *SriovNetworkPoolConfigReconciler) Reconcile(ctx context.Context, req ct } // remove our finalizer from the list and update it. var found bool - instance.ObjectMeta.Finalizers, found = sriovnetworkv1.RemoveString(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.ObjectMeta.Finalizers) + instance.Finalizers, found = sriovnetworkv1.RemoveString(sriovnetworkv1.POOLCONFIGFINALIZERNAME, instance.Finalizers) if found { if err := r.Update(ctx, instance); err != nil { return reconcile.Result{}, err diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index f79614c442..7f18066c31 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -94,13 +94,13 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl. snolog.SetLogLevel(defaultConfig.Spec.LogLevel) // examine DeletionTimestamp to determine if object is under deletion - if !defaultConfig.ObjectMeta.DeletionTimestamp.IsZero() { + if !defaultConfig.DeletionTimestamp.IsZero() { // The object is being deleted return r.handleSriovOperatorConfigDeletion(ctx, defaultConfig, logger) } // add finalizer if needed - if !sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) { - defaultConfig.ObjectMeta.Finalizers = append(defaultConfig.ObjectMeta.Finalizers, sriovnetworkv1.OPERATORCONFIGFINALIZERNAME) + if !sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.Finalizers) { + defaultConfig.Finalizers = append(defaultConfig.Finalizers, sriovnetworkv1.OPERATORCONFIGFINALIZERNAME) if err := r.Update(ctx, defaultConfig); err != nil { return reconcile.Result{}, err } @@ -449,7 +449,7 @@ func (r *SriovOperatorConfigReconciler) syncOpenShiftSystemdService(ctx context. func (r *SriovOperatorConfigReconciler) handleSriovOperatorConfigDeletion(ctx context.Context, defaultConfig *sriovnetworkv1.SriovOperatorConfig, logger logr.Logger) (ctrl.Result, error) { var err error - if sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) { + if sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.Finalizers) { // our finalizer is present, so lets handle any external dependency logger.Info("delete SriovOperatorConfig CR", "Namespace", defaultConfig.Namespace, "Name", defaultConfig.Name) // make sure webhooks objects are deleted prior of removing finalizer @@ -458,7 +458,7 @@ func (r *SriovOperatorConfigReconciler) handleSriovOperatorConfigDeletion(ctx co return reconcile.Result{}, err } // remove our finalizer from the list and update it. - defaultConfig.ObjectMeta.Finalizers, _ = sriovnetworkv1.RemoveString(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) + defaultConfig.Finalizers, _ = sriovnetworkv1.RemoveString(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.Finalizers) if err := r.Update(ctx, defaultConfig); err != nil { return reconcile.Result{}, err } diff --git a/deployment/sriov-network-operator-chart/README.md b/deployment/sriov-network-operator-chart/README.md index d5d529dc96..802d81b855 100644 --- a/deployment/sriov-network-operator-chart/README.md +++ b/deployment/sriov-network-operator-chart/README.md @@ -4,10 +4,12 @@ SR-IOV Network Operator Helm Chart provides an easy way to install, configure an the lifecycle of SR-IOV network operator. ## SR-IOV Network Operator + SR-IOV Network Operator leverages [Kubernetes CRDs](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/) and [Operator SDK](https://github.com/operator-framework/operator-sdk) to configure and manage SR-IOV networks in a Kubernetes cluster. SR-IOV Network Operator features: + - Initialize the supported SR-IOV NIC types on selected nodes. - Provision/upgrade SR-IOV device plugin executable on selected node. - Provision/upgrade SR-IOV CNI plugin executable on selected nodes. @@ -28,10 +30,11 @@ SR-IOV Network Operator features: ### Install Helm Helm provides an install script to copy helm binary to your system: -``` -$ curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3 -$ chmod 500 get_helm.sh -$ ./get_helm.sh + +```bash +curl -fsSL -o get_helm.sh https://raw.githubusercontent.com/helm/helm/master/scripts/get-helm-3 +chmod 500 get_helm.sh +./get_helm.sh ``` For additional information and methods for installing Helm, refer to the official [helm website](https://helm.sh/) @@ -40,13 +43,13 @@ For additional information and methods for installing Helm, refer to the officia #### Deploy from OCI repo -``` -$ helm install -n sriov-network-operator --create-namespace --version 1.3.0 --set sriovOperatorConfig.deploy=true sriov-network-operator oci://ghcr.io/k8snetworkplumbingwg/sriov-network-operator-chart +```bash +helm install -n sriov-network-operator --create-namespace --version 1.3.0 --set sriovOperatorConfig.deploy=true sriov-network-operator oci://ghcr.io/k8snetworkplumbingwg/sriov-network-operator-chart ``` #### Deploy from project sources -``` +```bash # Clone project $ git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git ; cd sriov-network-operator @@ -58,8 +61,9 @@ $ kubectl -n sriov-network-operator get pods ``` In the case that [Pod Security Admission](https://kubernetes.io/docs/concepts/security/pod-security-admission/) is enabled, the sriov network operator namespace will require a security level of 'privileged' -``` -$ kubectl label ns sriov-network-operator pod-security.kubernetes.io/enforce=privileged + +```bash +kubectl label ns sriov-network-operator pod-security.kubernetes.io/enforce=privileged ``` ## Chart parameters @@ -68,7 +72,7 @@ In order to tailor the deployment of the network operator to your cluster needs We have introduced the following Chart parameters. | Name | Type | Default | description | -| ---- |------|---------|-------------| +| ---- | ---- | ------- | ----------- | | `imagePullSecrets` | list | `[]` | An optional list of references to secrets to use for pulling any of the SR-IOV Network Operator image | | `supportedExtraNICs` | list | `[]` | An optional list of whitelisted NICs | @@ -97,21 +101,23 @@ The admission controllers can be enabled by switching on a single parameter `ope default, the user needs to pre-create Kubernetes Secrets that match the names provided in `operator.admissionControllers.certificates.secretNames`. The secrets should have 3 fields populated with the relevant content: -* `ca.crt` (value needs to be base64 encoded twice) -* `tls.crt` -* `tls.key` + +- `ca.crt` (value needs to be base64 encoded twice) +- `tls.crt` +- `tls.key` Aside from the aforementioned mode, the chart supports 3 more modes for certificate consumption by the admission controllers, which can be found in the table below. In a nutshell, the modes that are supported are: -* Consume pre-created Certificates managed by cert-manager -* Generate self signed Certificates managed by cert-manager -* Specify the content of the certificates as Helm values + +- Consume pre-created Certificates managed by cert-manager +- Generate self signed Certificates managed by cert-manager +- Specify the content of the certificates as Helm values | Name | Type | Default | description | | ---- | ---- | ------- | ----------- | | `operator.admissionControllers.enabled` | bool | false | Flag that switches on the admission controllers | | `operator.admissionControllers.certificates.secretNames.operator` | string | `operator-webhook-cert` | Secret that stores the certificate for the Operator's admission controller | -| `operator.admissionControllers.certificates.secretNames.injector` | string | `network-resources-injector-cert` | Secret that stores the certificate for the Network Resources Injector's admission controller | +| `operator.admissionControllers.certificates.secretNames.injector` | string | `network-resources-injector-cert` | Secret that stores the certificate for the Network Resources Injector's admission controller | | `operator.admissionControllers.certificates.certManager.enabled` | bool | false | Flag that switches on consumption of certificates managed by cert-manager | | `operator.admissionControllers.certificates.certManager.generateSelfSigned` | bool | false | Flag that switches on generation of self signed certificates managed by cert-manager. The secrets in which the certificates are stored will have the names provided in `operator.admissionControllers.certificates.secretNames` | | `operator.admissionControllers.certificates.custom.enabled` | bool | false | Flag that switches on consumption of user provided certificates that are part of `operator.admissionControllers.certificates.custom.operator` and `operator.admissionControllers.certificates.custom.injector` objects | @@ -135,7 +141,7 @@ This section contains general parameters that apply to both the operator and dae | `sriovOperatorConfig.configurationMode` | string | `daemon` | sriov-network-config-daemon configuration mode. either `daemon` or `systemd` | | `sriovOperatorConfig.featureGates` | map[string]bool | `{}` | feature gates to enable/disable | -**Note** +#### Note When `sriovOperatorConfig.configurationMode` is configured as `systemd`, configurations files and `systemd` service files are created on the node. Upon chart deletion, those files are not cleaned up. For cases where this is not acceptable, users should rather configured the `daemon` mode. @@ -149,7 +155,7 @@ Upon chart deletion, those files are not cleaned up. For cases where this is not | `images.sriovCni` | SR-IOV CNI image | | `images.ibSriovCni` | InfiniBand SR-IOV CNI image | | `images.ovsCni` | OVS CNI image | -| `images.rdmaCni` | RDMA CNI image | +| `images.rdmaCni` | RDMA CNI image | | `images.sriovDevicePlugin` | SR-IOV device plugin image | | `images.resourcesInjector` | Resources Injector image | | `images.webhook` | Operator Webhook image | @@ -163,5 +169,5 @@ Upon chart deletion, those files are not cleaned up. For cases where this is not Please note that any resources deployed using the `extraDeploy` in this Helm chart are the sole responsibility of the user. It is important to review and understand the implications of these deployed resources. The maintainers of this Helm chart take no responsibility for any issues or damages caused by the deployment or operation of these resources. | Name | description | -| ---- | ------------| -|`extraDeploy`| Array of extra objects to deploy with the release | +| ---- | ----------- | +| `extraDeploy` | Array of extra objects to deploy with the release | diff --git a/doc/design/externally-manage-pf.md b/doc/design/externally-manage-pf.md index def3b98687..614e0704db 100644 --- a/doc/design/externally-manage-pf.md +++ b/doc/design/externally-manage-pf.md @@ -1,5 +1,4 @@ --- -title: Externally Manage PF authors: - SchSeba reviewers: @@ -13,8 +12,9 @@ last-updated: 12-07-2023 ## Summary -Allow the SR-IOV network operator to configure and allocate a subset of virtual functions from -a physical function that is configured externally from SR-IOV network operator. +Allow the SR-IOV network operator to configure and allocate a subset of +virtual functions from a physical function that is configured externally from +SR-IOV network operator. ## Motivation @@ -59,6 +59,7 @@ Then the operator will configure the subset of virtual functions with the reques configmap with the expected information to create the relevant pools. Existing sriov network config daemon flow: + 1. Apply the `numOfVfs` 2. Configure the MTU on the PF 3. Copy the Administrative mac address from the VFs @@ -66,11 +67,13 @@ Existing sriov network config daemon flow: 5. restart sriov network device plugin Externally manage sriov network config daemon flow: + 1. Copy the Administrative mac address from the VFs 2. Bind the right driver for the VF 3. restart sriov network device plugin In both flows: + * In case of Infiniband link type it will generate random node and port GUID for the interface. * In case of RDMA (both for ETH and IB) it will perform an unbind/bind of the VF driver to set RDMA Node/Port GUID. @@ -88,7 +91,8 @@ If the user want to create the virtual functions after the SR-IOV Network config to disable the webhook. the policy will be on failed state until the virtual functions needed for the policy exist on the node. the SR-IOV Network config daemon will continue to reconcile until the virtual functions exists -#### Policy Example: +#### Policy Example + ```yaml apiVersion: sriovnetwork.openshift.io/v1 kind: SriovNetworkNodePolicy @@ -107,10 +111,11 @@ spec: externallyManaged: true ``` -The PF and VFs 0-4 are externally managed. +The PF and VFs 0-4 are externally managed. For example nmstate will create 10 vfs, but will only consume VF 0 and 4 in its configuration. Nmstate will also manage the MTU and other parameters of the PF. -#### Another Policy Example: +#### Another Policy Example + In this case we allocate all the virtual functions from the PF ```yaml @@ -136,19 +141,18 @@ One if the main use cases for this is if the user want to do some custom configu out of tree drivers or other stuff that the operator doesn't support. #### Validation + The SR-IOV network operator will do a validation webhook to check if the requested `numVfs` is equal to what the user allocate if not it will reject the policy creation. -The SR-IOV network operator will do a validation webhook to check if the requested MTU is lower or equal to what exist on the PF +The SR-IOV network operator will do a validation webhook to check if the requested MTU is lower or equal to what exist on the PF if not it will reject the policy creation. - *Note:* Same validation will be done in the SR-IOV config-daemon container to cover cases where the user doesn't want to deploy" the webhook and to cover scale-up adding new nodes. If the verification failed in the policy apply stage -the `sriovNetworkNodeState.status.SyncStatus` field will be report a `Failed` status and the error description will +the `sriovNetworkNodeState.status.SyncStatus` field will be report a `Failed` status and the error description will get exposed in `sriovNetworkNodeState.status.LastSyncError` - #### Configuration The SR-IOV network operator config daemon will reconcile on the SriovNetworkNodeState update and will follow the regular @@ -184,7 +188,9 @@ type Interface struct { ### Implementation Details/Notes/Constraints #### Webhook + For the webhook we add more validations when the policy contains `ExternallyManaged: true` + * `numVfs` in the policy equal is equal or lower the number of virtual functions on the system * `MTU` in the policy equals or lower the MTU we discover on the PF * `LinkType` in the policy equals the link type we discover on the PF @@ -200,8 +206,8 @@ This is where most of the changes for this feature are implemented. * do a validation same as on the webhook to check the PF have everything we need to apply the requested policy, by checking the `numVfs`, `MTU` and `LinkType`. -* skip all the PF configuration like `numVfs`, `MTU` and `LinkType`. he will only perform the virtual function -driver binding, administrative mac allocation and MTU. +* skip all the PF configuration like `numVfs`, `MTU` and `LinkType`. he will only perform the virtual function +driver binding, administrative mac allocation and MTU. * in case of Infiniband link type it will generate random node and port GUID for the interface * in case of RDMA (both for ETH and IB) it will perform an unbind/bind of the VF driver to set RDMA Node/Port GUID. * reset the device plugin so kubelet will be able to discover the SR-IOV devices. @@ -221,4 +227,4 @@ this may cause conflicts in the system. * Should create a policy if the number of requested vfs is equal * Should create a policy if the number of requested vfs is equal and not delete them when the policy is removed * should reset the virtual functions if externallyCreated is false -* should to configure a policy with externallyManaged true if there are no vfs configured with disabled webhook \ No newline at end of file +* should to configure a policy with externallyManaged true if there are no vfs configured with disabled webhook diff --git a/doc/design/ib-vf-configuration.md b/doc/design/ib-vf-configuration.md index 74f56f3c61..46ce15c1df 100644 --- a/doc/design/ib-vf-configuration.md +++ b/doc/design/ib-vf-configuration.md @@ -1,5 +1,4 @@ --- -title: IB VF GUID Configuration authors: - almaslennikov reviewers: @@ -11,9 +10,11 @@ last-updated: 13-03-2024 # IB VF GUID Configuration ## Summary + Allow SR-IOV Network Operator to use a static configuration file from the host filesystem to assign GUIDs to IB VFs ## Motivation + We have customers using the SR-IOV operator to create IB VFs, and they need a way to automate GUID assignment, so that IB VFs are automatically bound to the required PKeys and no additional manual configuration is needed. We would like SR-IOV Network Operator to configure VFs with the set of assigned guids based on provided configuration. @@ -30,7 +31,6 @@ Now the GUIDs are assigned by the sriov-network-config-daemon randomly. * Dynamic GUID allocation is out of scope of this proposal - ### Assumptions * Per node IB GUID configuration is static and created in advance @@ -126,7 +126,7 @@ The alternative solution is also based on the GUID configuration file being depl The difference here is that GUID assignment is done on the cni level when a VF is allocated to a pod. ib-sriov-cni manages a host-local per-PF pool of allocated/free GUIDs and dynamically allocates the next free GUID to an allocated VF. -### Workflow: +### Workflow 1. A script is deployed to the host and creates a static GUID config file. This step is out of scope of the operator and can also be carried out manually. The script would usually need to be custom and based on the specific GUID provisioning system in place. 2. SR-IOV network operator creates IB VFs with random GUIDs (as done now) @@ -139,11 +139,13 @@ ib-sriov-cni manages a host-local per-PF pool of allocated/free GUIDs and dynami ## Comparison between the two alternatives The SR-IOV Network Operator approach: + * Easier to implement and less error-prone * Manages the whole lifecycle of the VF (GUID is assigned at creation and never changes throughout the lifecycle) * Operator has better visibility into the amount of configured VFs The IB-SRIOV-CNI approach: + * Offers more flexibility (Only when a VF is requested for an IB network will it be assigned a GUID) * Easier to maintain complex use cases - * 2 PFs on the node evenly split between 2 PKeys. The CNI approach will require 2 per-PF resource pools and 4 network attachments. The operator approach will require 4 resource pools and 4 network attachments, one for each PKey-PF pair. \ No newline at end of file + * 2 PFs on the node evenly split between 2 PKeys. The CNI approach will require 2 per-PF resource pools and 4 network attachments. The operator approach will require 4 resource pools and 4 network attachments, one for each PKey-PF pair. diff --git a/doc/design/parallel-node-config.md b/doc/design/parallel-node-config.md index b949267c2a..82b4b812f1 100644 --- a/doc/design/parallel-node-config.md +++ b/doc/design/parallel-node-config.md @@ -1,5 +1,4 @@ --- -title: Parallel SR-IOV configuration authors: - SchSeba reviewers: @@ -12,9 +11,11 @@ last-updated: 18-07-2023 # Parallel SR-IOV configuration ## Summary + Allow SR-IOV Network Operator to configure more than one node at the same time. ## Motivation + SR-IOV Network Operator configures SR-IOV one node at a time and one nic at a same time. That means we’ll need to wait hours or even days to configure all NICs on large cluster deployments. Also moving all draining logic to a centralized place which will reduce chances of race conditions and bugs that were encountered before in sriov-network-config-daemon @@ -23,20 +24,22 @@ with draining. ### Use Cases ### Goals + * Number of drainable nodes should be 1 by default * Number of drainable nodes should be configured by pool * Nodes pool should be defined by node selector * Move all drain-related logic into the centralized place - ### Non-Goals + Parallel NICs configuration on the same node is out of scope of this proposal ## Proposal -Introduce nodes pool drain configuration and controller to meet goals targets. +Introduce nodes pool drain configuration and controller to meet goals targets. ### Workflow Description + A new Drain controller will be introduced to manage node drain and cordon procedures. That means we don't need to do drain and use `drain lock` in config daemon anymore. The overall drain process will be covered by the following states: @@ -51,7 +54,7 @@ Draining = "Draining" DrainComplete = "DrainComplete" ``` -Drain controller will watch for Node annotation, `sriovnetwork.openshift.io/state` +Drain controller will watch for Node annotation, `sriovnetwork.openshift.io/state` and SriovNetworkNodeState annotation `sriovnetwork.openshift.io/desired-state` and write the `sriovnetwork.openshift.io/current-state` annotation in the SriovNetworkNodeState. @@ -68,18 +71,19 @@ Draining procedure: 1. if number of `Draining` nodes is great or equal to the `MaxUnavailable` the operator will re-queue the request 2. if number of `Draining` nodes is lower than the `MaxUnavailable` the operator will start the draining process and annotate the SriovNetworkNodeState annotation `sriovnetwork.openshift.io/current-state` with `Draining` -5. on Openshift platform we will pause the machine config pool related to the node -6. the operator will start the drain process +3. on Openshift platform we will pause the machine config pool related to the node +4. the operator will start the drain process 1. if `Drain_Required` the operator will remove ONLY pods used sriov devices 2. if `Reboot_Required` the operator will remove ALL the pods on the system -9. operator moves the `sriovnetwork.openshift.io/current-state` annotation to `DrainComplete` -10. daemon will continue to the configuration when it's done it will move back both `sriovnetwork.openshift.io/state` +5. operator moves the `sriovnetwork.openshift.io/current-state` annotation to `DrainComplete` +6. daemon will continue to the configuration when it's done it will move back both `sriovnetwork.openshift.io/state` annotation on Node and `sriovnetwork.openshift.io/desired-state` on SriovNetworkNodeState to `Idle` -11. operator runs the complete drain to remove the cordon and mark the `sriovnetwork.openshift.io/current-state` annotation to `Idle` +7. operator runs the complete drain to remove the cordon and mark the `sriovnetwork.openshift.io/current-state` annotation to `Idle` ### API Extensions #### Extend existing CR SriovNetworkPoolConfig + SriovNetworkPoolConfig is used only for OpenShift to provide configuration for OVS Hardware Offloading. We can extend it to add configuration for the drain pool. E.g.: @@ -88,7 +92,7 @@ pool. E.g.: // SriovNetworkPoolConfigSpec defines the desired state of SriovNetworkPoolConfig type SriovNetworkPoolConfigSpec struct { ... - + // nodeSelector specifies a label selector for Nodes NodeSelector *metav1.LabelSelector `json:"nodeSelector,omitempty"` @@ -130,19 +134,22 @@ and OpenShift clusters. Node can only be part of one pool. if the node is not part of any node it will be allocated to a virtual default pool with `maxUnavailable` of 1. -_*Note:*_ if you create a pool with empty selector it will match all the nodes, and you can not have another pool. +**Note:** if you create a pool with empty selector it will match all the nodes, and you can not have another pool. ### Upgrade & Downgrade considerations + After operator upgrade we have to support `sriovnetwork.openshift.io/state` node annotation and `sriovnetwork.openshift.io/desired-state` annotation in the `sriovNetworkNodeState`. in the future we are going to migrate to only using the annotation in the `sriovNetworkNodeState` There is no change in upgrade from the user point of view. If there is no pools or the node doesn't belong to any pool the `maxUnavailable` will be 1 to preserve the same functionality after upgrade. -_*Note:*_ no node should be in `Draining` or `MCP_Paused` state in the node annotation before the upgrade +**Note:** no node should be in `Draining` or `MCP_Paused` state in the node annotation before the upgrade ### Alternative APIs + #### Option 1: extend SriovOperatorConfig CRD + We can extend SriovOperatorConfig CRD to include drain pools configuration. E.g.: ```yaml @@ -181,8 +188,10 @@ We didn't choose this option because SriovOperatorConfig contains Config Daemon- configuration is node-specific. #### Option 2: New CRD + Add new `DrainConfiguration`CRD with fields mentioned in previous options. We can extend SriovOperatorConfig CRD to include drain pools configuration. E.g.: + ```yaml apiVersion: sriovnetwork.openshift.io/v1 kind: SriovDrainConfig @@ -203,7 +212,8 @@ We didn't choose this option because there is already defined `SriovNetworkPoolC configuration. ### Test Plan + * Unit tests will be implemented for new Drain Controller. -** E2E, manual or automation functional testing should have such test cases: +**E2E, manual or automation functional testing should have such test cases: ** to verify that we actually configure SR-IOV on `MaxParallelNodeConfiguration` nodes at the same time ** to check that we don't configure more than `MaxParallelNodeConfiguration` nodes at the same time diff --git a/doc/design/software-bridge-management.md b/doc/design/software-bridge-management.md index 50fa0ad7ba..f99dcdf373 100644 --- a/doc/design/software-bridge-management.md +++ b/doc/design/software-bridge-management.md @@ -1,5 +1,4 @@ --- -title: software bridge management authors: - ykulazhenkov reviewers: @@ -11,65 +10,84 @@ last-updated: 15-02-2024 ## Summary -When NIC is configured to switchdev mode, a VF representor net device is created for each VF on it. -These representors are used by a software switch (OVS, Linux bridge) to control traffic and configure hardware offloads. -The software bridge is an essential part of using NICs in switchdev mode. +When NIC is configured to switchdev mode, a VF representor net device +is created for each VF on it. These representors are used by a software +switch (OVS, Linux bridge) to control traffic and configure hardware +offloads. The software bridge is an essential part of using NICs in +switchdev mode. -**sriov-network-operator** can set switchdev mode for a NIC and create VFs on it, -but it doesn't provide any functionality to create and configure software bridges. +**sriov-network-operator** can set switchdev mode for a NIC and create +VFs on it, but it doesn't provide any functionality to create and +configure software bridges. -This document contains a proposal to add limited support for software bridges configuration to the **sriov-network-operator**. +This document contains a proposal to add limited support for software +bridges configuration to the **sriov-network-operator**. -This feature assumes integration with [ovs-cni](https://github.com/k8snetworkplumbingwg/ovs-cni) and +This feature assumes integration with [ovs-cni](https://github.com/k8snetworkplumbingwg/ovs-cni) and [accelerated-bridge-cni](https://github.com/k8snetworkplumbingwg/accelerated-bridge-cni). Depends on [_switchdev and systemd modes refactoring_](switchdev-refactoring.md) feature. ## Motivation -SRIOV Legacy mode is no longer actively developed, and we need to encourage users to migrate to switchdev mode, -which is actively developed and will continue to receive new features and improvements. - -To promote switching to switchdev configurations, we need to provide a nice UX for the end user. -This requires providing an easy way to configure software switches, which are prerequisites for NICs in switchdev mode. +SRIOV Legacy mode is no longer actively developed, and we need to +encourage users to migrate to switchdev mode, which is actively +developed and will continue to receive new features and improvements. +To promote switching to switchdev configurations, we need to provide a +nice UX for the end user. This requires providing an easy way to +configure software switches, which are prerequisites for NICs in +switchdev mode. ### Use Cases -* As a user, I expect that **sriov-network-operator** will install `ovs-cni` and `accelerated-bridge-cni` to hosts. -* As a user, I want to create `OVSNetwork` CR, which will result in creation of `NetworkAttachmentDefinition` CR that uses -`ovs-cni` and contains required resource request. -* As a user, I want to create `BridgeNetwork` CR which will result in creation of `NetworkAttachmentDefinition` CR that uses -`accelerated-bridge-cni` and contains the required resource requests. -* As a user, I want to define configuration for software bridges inside the `SriovNetworkNodePolicy` CR and expect that the -operator will create required bridges, configure them, and attach uplinks (physical functions). +* As a user, I expect that **sriov-network-operator** will install + `ovs-cni` and `accelerated-bridge-cni` to hosts. +* As a user, I want to create `OVSNetwork` CR, which will result in + creation of `NetworkAttachmentDefinition` CR that uses `ovs-cni` and + contains required resource request. +* As a user, I want to create `BridgeNetwork` CR which will result in + creation of `NetworkAttachmentDefinition` CR that uses + `accelerated-bridge-cni` and contains the required resource requests. +* As a user, I want to define configuration for software bridges inside + the `SriovNetworkNodePolicy` CR and expect that the operator will + create required bridges, configure them, and attach uplinks + (physical functions). ### Goals * handle installation of `ovs-cni` and `accelerated-bridge-cni` -* add `OVSNetwork` and `BridgeNetwork` CRDs as an API for end users to simplify creation of `NetworkAttachmentDefinition` CR -for `ovs-cni` and `accelerated-bridge-cni` -* extend `SriovNetworkNodePolicy` CRD to support configuration of software bridges (bridge-level configuration) -* extend `SriovNetworkNodeState` (spec and status) CRD to support configuration of software bridges (bridge-level configuration) -* support configuration of software bridge in both modes (operator's `configurationMode` setting): `daemon` and `systemd` -* implementation should be compatible with [_Externally Managed PF_](externally-manage-pf.md) feature - +* add `OVSNetwork` and `BridgeNetwork` CRDs as an API for end users to + simplify creation of `NetworkAttachmentDefinition` CR for `ovs-cni` + and `accelerated-bridge-cni` +* extend `SriovNetworkNodePolicy` CRD to support configuration of + software bridges (bridge-level configuration) +* extend `SriovNetworkNodeState` (spec and status) CRD to support + configuration of software bridges (bridge-level configuration) +* support configuration of software bridge in both modes (operator's + `configurationMode` setting): `daemon` and `systemd` +* implementation should be compatible with + [_Externally Managed PF_](externally-manage-pf.md) feature ### Non-Goals * replace `SriovNetworkPoolConfig` CRD * change API for host-level settings, e.g. `ovs-hw-offload` - - _**Note:** we may need to extend this API to support additional options_ + + _**Note:** we may need to extend this API to support additional options_ * add support for VF-lag use-case ## Proposal -1. deploy `ovs-cni` and `accelerated-bridge-cni` with init containers of `sriov-network-config-daemon` Pod -2. define `OVSNetwork` and `BridgeNetwork` CRDs and implement controllers for them which will create `NetworkAttachmentDefinition` CRs -3. extend `SriovNetworkNodePolicy`and `SriovNetworkNodeState` (spec and status) CRDs to support configuration of software bridges (bridge-level configuration) - +1. deploy `ovs-cni` and `accelerated-bridge-cni` with init containers + of `sriov-network-config-daemon` Pod +2. define `OVSNetwork` and `BridgeNetwork` CRDs and implement + controllers for them which will create `NetworkAttachmentDefinition` + CRs +3. extend `SriovNetworkNodePolicy` and `SriovNetworkNodeState` (spec + and status) CRDs to support configuration of software bridges + (bridge-level configuration) ### Workflow Description @@ -81,71 +99,88 @@ Implementation should be compatible with the following workflows: #### Fully automatic workflow -This workflow assumes that **sriov-network-operator** handles PFs and VFs configuration, creation and configuration of software bridge, announcement of SRIOV resources with device plugin and preparation of `NetworkAttachmentDefinition` CR. +This workflow assumes that **sriov-network-operator** handles PFs and +VFs configuration, creation and configuration of software bridge, +announcement of SRIOV resources with device plugin and preparation of +`NetworkAttachmentDefinition` CR. 1. User creates `SriovNetworkNodePolicy` where: * eswitch mode set to `switchdev` * configuration for selected software bridge is defined 2. The operator populates `SriovNetworkNodeState` for matching nodes with PF and bridge -configurations - -3. `sriov-network-config-daemon` applies PF configuration, create and configure bridge, attach PF to the bridge, applies VF configuration +configurations +3. `sriov-network-config-daemon` applies PF configuration, create and + configure bridge, attach PF to the bridge, applies VF configuration - _**Note 1:** if the operator runs in the `systemd` mode then bridge creation should happen in the `pre` phase._ + _**Note 1:** if the operator runs in the `systemd` mode then bridge + creation should happen in the `pre` phase._ - _**Note 2:** we should create udev rule which will set `NM_UNMANAGED=1` for bridges created by the operator_ + _**Note 2:** we should create udev rule which will set + `NM_UNMANAGED=1` for bridges created by the operator_ -4. `sriov-network-config-daemon` should report information about software bridges in the status field of the `SriovNetworkNodeState` CR. +4. `sriov-network-config-daemon` should report information about + software bridges in the status field of the + `SriovNetworkNodeState` CR. 5. SRIOV resources are announced by the Device plugin -6. User creates `OVSNetwork` or `BridgeNetwork` CR to create `NetworkAttachmentDefinition` CR, which relies on resources announced by the Device Plugin - - -_**Note:** created software bridge should be removed during the PF configuration reset_ +6. User creates `OVSNetwork` or `BridgeNetwork` CR to create + `NetworkAttachmentDefinition` CR, which relies on resources announced + by the Device Plugin +_**Note:** created software bridge should be removed during the PF +configuration reset_ #### NIC configuration only flow -This workflow is kept to support existing HW offloading use-case. +This workflow is kept to support existing HW offloading use-case. -In this case, **sriov-network-operator** handles PFs and VFs configuration, announcement of SRIOV resources with device plugin and preparation of `NetworkAttachmentDefinition` CR. +In this case, **sriov-network-operator** handles PFs and VFs +configuration, announcement of SRIOV resources with device plugin and +preparation of `NetworkAttachmentDefinition` CR. -In some scenarios, it may be compatible with `configurationMode: daemon`, but it is supposed to be used when the operator runs in `systemd` mode. +In some scenarios, it may be compatible with `configurationMode: daemon`, +but it is supposed to be used when the operator runs in `systemd` mode. 1. User creates `SriovNetworkNodePolicy` where: * eswitch mode set to `switchdev` _**Note:** `SriovNetworkNodePolicy` CR should not include bridge configuration_ - 2. The operator populates `SriovNetworkNodeState` for matching nodes with PF configurations 3. `pre` systemd service creates VFs and configure PF 4. NetworkManager or systemd-networkd or environment-specific scripts create bridge -5. `post` systemd service binds VFs to required driver and proceed with other configuration steps +5. `post` systemd service binds VFs to required driver and proceed with + other configuration steps 6. SRIOV resources are announced by the Device plugin -7. User creates `OVSNetwork` or `BridgeNetwork` CR to create `NetworkAttachmentDefinition` CR which relies on resource announced by the Device Plugin - -_**Note:** it is possible to use externally created `NetworkAttachmentDefinition` CR that contains configuration for any CNI plugin that support -resources from NICs in switchdev mode._ +7. User creates `OVSNetwork` or `BridgeNetwork` CR to create + `NetworkAttachmentDefinition` CR which relies on resource announced + by the Device Plugin +_**Note:** it is possible to use externally created +`NetworkAttachmentDefinition` CR that contains configuration for any +CNI plugin that support resources from NICs in switchdev mode._ _**Note 1:** bridge is not removed during the PF configuration reset_ -_**Note 2:** information about bridges is not reported in the status field of the `SriovNetworkNodeState` CR_ +_**Note 2:** information about bridges is not reported in the status +field of the `SriovNetworkNodeState` CR_ #### Externally managed NIC flow -In this case, **sriov-network-operator** handles announcement of SRIOV resources with device plugin and preparation of `NetworkAttachmentDefinition` CR. +In this case, **sriov-network-operator** handles announcement of SRIOV +resources with device plugin and preparation of +`NetworkAttachmentDefinition` CR. -1. User configures PFs, creates VFs, creates bridge and attaches PFs to the bridge with custom scripts. +1. User configures PFs, creates VFs, creates bridge and attaches PFs + to the bridge with custom scripts. 2. User creates `SriovNetworkNodePolicy` where: * eswitch mode set to `switchdev` @@ -154,9 +189,11 @@ In this case, **sriov-network-operator** handles announcement of SRIOV resources 3. `sriov-network-config-daemon` configure VFs -3. SRIOV resources are announced by the Device plugin +4. SRIOV resources are announced by the Device plugin -4. User creates `OVSNetwork` or `BridgeNetwork` CR to create `NetworkAttachmentDefinition` CR, which relies on resources announced by the Device Plugin +5. User creates `OVSNetwork` or `BridgeNetwork` CR to create + `NetworkAttachmentDefinition` CR, which relies on resources announced + by the Device Plugin ### API Extensions @@ -168,7 +205,6 @@ In this case, **sriov-network-operator** handles announcement of SRIOV resources | `ACCELERATED_BRIDGE_CNI_IMAGE` | contains full image name for `accelerated-bridge-cni` | | `OVSDB_SOCKET_PATH` | path to the OVSDB socket, used to path to the sriov config daemon | - #### Feature flags `manageSoftwareBridges` - control state of the feature (default: `false`) @@ -184,35 +220,35 @@ In this case, **sriov-network-operator** handles announcement of SRIOV resources ```golang // OVSNetworkSpec defines the desired state of OvsNetwork type OVSNetworkSpec struct { - // Namespace of the NetworkAttachmentDefinition custom resource - NetworkNamespace string `json:"networkNamespace,omitempty"` - // OVS Network device plugin endpoint resource name - ResourceName string `json:"resourceName"` - // Capabilities to be configured for this network. - // Capabilities supported: (mac|ips), e.g. '{"mac": true}' - Capabilities string `json:"capabilities,omitempty"` - // IPAM configuration to be used for this network. - IPAM string `json:"ipam,omitempty"` - // name of the OVS bridge, if not set OVS will automatically select bridge - // based on VF PCI address - Bridge string `json:"bridge,omitempty"` - // Vlan to assign for the OVS port - Vlan uint `json:"vlan,omitempty"` - // Mtu for the OVS port - MTU uint `json:"mtu",omitempty` - // Trunk configuration for the OVS port - Trunk []*TrunkConfig `json:"trunk,omitempty"` - // The type of interface on ovs. - InterfaceType string `json:"interfaceType,omitempty"` - // MetaPluginsConfig configuration to be used in order to chain metaplugins - MetaPluginsConfig string `json:"metaPlugins,omitempty"` + // Namespace of the NetworkAttachmentDefinition custom resource + NetworkNamespace string `json:"networkNamespace,omitempty"` + // OVS Network device plugin endpoint resource name + ResourceName string `json:"resourceName"` + // Capabilities to be configured for this network. + // Capabilities supported: (mac|ips), e.g. '{"mac": true}' + Capabilities string `json:"capabilities,omitempty"` + // IPAM configuration to be used for this network. + IPAM string `json:"ipam,omitempty"` + // name of the OVS bridge, if not set OVS will automatically select bridge + // based on VF PCI address + Bridge string `json:"bridge,omitempty"` + // Vlan to assign for the OVS port + Vlan uint `json:"vlan,omitempty"` + // Mtu for the OVS port + MTU uint `json:"mtu",omitempty` + // Trunk configuration for the OVS port + Trunk []*TrunkConfig `json:"trunk,omitempty"` + // The type of interface on ovs. + InterfaceType string `json:"interfaceType,omitempty"` + // MetaPluginsConfig configuration to be used in order to chain metaplugins + MetaPluginsConfig string `json:"metaPlugins,omitempty"` } // OVSTrunkConfig contains configuration for OVS trunk type TrunkConfig struct { - MinID *uint `json:"minID,omitempty"` - MaxID *uint `json:"maxID,omitempty"` - ID *uint `json:"id,omitempty"` + MinID *uint `json:"minID,omitempty"` + MaxID *uint `json:"maxID,omitempty"` + ID *uint `json:"id,omitempty"` } // OvsNetworkStatus defines the observed state of OvsNetwork @@ -221,11 +257,11 @@ type OvsNetworkStatus struct { // OvsNetwork is the Schema for the ovsnetworks API type OvsNetwork struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` - Spec OvsNetworkSpec `json:"spec,omitempty"` - Status OvsNetworkStatus `json:"status,omitempty"` + Spec OvsNetworkSpec `json:"spec,omitempty"` + Status OvsNetworkStatus `json:"status,omitempty"` } ``` @@ -235,28 +271,28 @@ type OvsNetwork struct { ```golang // BridgeNetworkSpec defines the desired state of BridgeNetwork type BridgeNetworkSpec struct { - // Namespace of the NetworkAttachmentDefinition custom resource - NetworkNamespace string `json:"networkNamespace,omitempty"` - // OVS Network device plugin endpoint resource name - ResourceName string `json:"resourceName"` - // Capabilities to be configured for this network. - // Capabilities supported: (mac|ips), e.g. '{"mac": true}' - Capabilities string `json:"capabilities,omitempty"` - // IPAM configuration to be used for this network. - IPAM string `json:"ipam,omitempty"` - // name of the Linux bridge, if not set will automatically select bridge - // based on VF PCI address - Bridge string `json:"bridge,omitempty"` - // VLAN ID for VF - Vlan uint `json:"vlan,omitempty"` - // VLAN Trunk configuration - Trunk []TrunkConfig `json:"trunk,omitempty"` - // enable setting matching vlan tags on the bridge uplink interface, default is false - SetUplinkVlan bool `json:"setUplinkVlan,omitempty"` - // MTU for VF and representor - MTU uint `json:"mtu,omitempty"` - // MetaPluginsConfig configuration to be used in order to chain metaplugins - MetaPluginsConfig string `json:"metaPlugins,omitempty"` + // Namespace of the NetworkAttachmentDefinition custom resource + NetworkNamespace string `json:"networkNamespace,omitempty"` + // OVS Network device plugin endpoint resource name + ResourceName string `json:"resourceName"` + // Capabilities to be configured for this network. + // Capabilities supported: (mac|ips), e.g. '{"mac": true}' + Capabilities string `json:"capabilities,omitempty"` + // IPAM configuration to be used for this network. + IPAM string `json:"ipam,omitempty"` + // name of the Linux bridge, if not set will automatically select bridge + // based on VF PCI address + Bridge string `json:"bridge,omitempty"` + // VLAN ID for VF + Vlan uint `json:"vlan,omitempty"` + // VLAN Trunk configuration + Trunk []TrunkConfig `json:"trunk,omitempty"` + // enable setting matching vlan tags on the bridge uplink interface, default is false + SetUplinkVlan bool `json:"setUplinkVlan,omitempty"` + // MTU for VF and representor + MTU uint `json:"mtu,omitempty"` + // MetaPluginsConfig configuration to be used in order to chain metaplugins + MetaPluginsConfig string `json:"metaPlugins,omitempty"` } // BridgeNetworkStatus defines the observed state of BridgeNetwork @@ -265,20 +301,20 @@ type BridgeNetworkStatus struct { // BridgeNetwork is the Schema for the ovsnetworks API type BridgeNetwork struct { - metav1.TypeMeta `json:",inline"` - metav1.ObjectMeta `json:"metadata,omitempty"` + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` - Spec BridgeNetworkSpec `json:"spec,omitempty"` - Status BridgeNetworkStatus `json:"status,omitempty"` + Spec BridgeNetworkSpec `json:"spec,omitempty"` + Status BridgeNetworkStatus `json:"status,omitempty"` } //+kubebuilder:object:root=true // BridgeNetworkList contains a list of BridgeNetwork type BridgeNetworkList struct { - metav1.TypeMeta `json:",inline"` - metav1.ListMeta `json:"metadata,omitempty"` - Items []BridgeNetwork `json:"items"` + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []BridgeNetwork `json:"items"` } ``` @@ -288,60 +324,60 @@ type BridgeNetworkList struct { ```golang // SriovNetworkNodePolicySpec defines the desired state of SriovNetworkNodePolicy type SriovNetworkNodePolicySpec struct { - // ...existing fields... - // contains spec for the software bridge - Bridge Bridge `json:"bridge,omitempty"` + // ...existing fields... + // contains spec for the software bridge + Bridge Bridge `json:"bridge,omitempty"` } // contains spec for the bridge // only one bridge type can be set type Bridge struct { - // contains optional config for OVS bridge - Ovs *OVSConfig `json:"ovs,omitempty"` - // contains optional config for Linux bridge - Linux *LinuxBridgeConfig `json:"linux,omitempty"` + // contains optional config for OVS bridge + Ovs *OVSConfig `json:"ovs,omitempty"` + // contains optional config for Linux bridge + Linux *LinuxBridgeConfig `json:"linux,omitempty"` } // OVSConfig optional configuration for OVS bridge and uplink Interface type OVSConfig struct { - // contains bridge level settings - Bridge OVSBridgeConfig `json:"bridge,omitempty"` - // contains settings for uplink (PF) - Uplink OVSUplinkConfig `json:"uplink,omitempty"` + // contains bridge level settings + Bridge OVSBridgeConfig `json:"bridge,omitempty"` + // contains settings for uplink (PF) + Uplink OVSUplinkConfig `json:"uplink,omitempty"` } // OVSBridgeConfig contains some options from the Bridge table in OVSDB type OVSBridgeConfig struct { - DatapathType string `json:"datapathType,omitempty"` - ExternalIDs map[string]string `json:"externalIDs,omitempty"` - OtherConfig map[string]string `json:"otherConfig,omitempty"` + DatapathType string `json:"datapathType,omitempty"` + ExternalIDs map[string]string `json:"externalIDs,omitempty"` + OtherConfig map[string]string `json:"otherConfig,omitempty"` } // OVSUplinkConfig contains PF interface configuration for the bridge type OVSUplinkConfig struct { - Interface OVSInterfaceConfig `json:"interface,omitempty"` - // can be extended to support OVSPortConfig which will include - // settings from the OVS Port table + Interface OVSInterfaceConfig `json:"interface,omitempty"` + // can be extended to support OVSPortConfig which will include + // settings from the OVS Port table } // OVSInterfaceConfig contains some options from the Interface table of the OVSDB for PF type OVSInterfaceConfig struct { - Type string `json:"type,omitempty"` - Options map[string]string `json:"options,omitempty"` - ExternalIDs map[string]string `json:"externalIDs,omitempty"` - OtherConfig map[string]string `json:"otherConfig,omitempty"` + Type string `json:"type,omitempty"` + Options map[string]string `json:"options,omitempty"` + ExternalIDs map[string]string `json:"externalIDs,omitempty"` + OtherConfig map[string]string `json:"otherConfig,omitempty"` } // LinuxBridgeConfig optional configuration for Linux bridge and uplink interface type LinuxBridgeConfig struct { - Bridge BridgeConfig `json:"bridge,omitempty"` - Uplink map[string]string `json:"uplink,omitempty"` // TODO clarify required settings + Bridge BridgeConfig `json:"bridge,omitempty"` + Uplink map[string]string `json:"uplink,omitempty"` // TODO clarify required settings } // BridgeConfig contains some options for linux bridge type BridgeConfig struct { - VlanFiltering bool `json:"vlanFiltering,omitempty"` - // +kubebuilder:validation:Enum=802.1Q;802.1ad - VlanProtocol string `json:"vlanProtocol,omitempty"` + VlanFiltering bool `json:"vlanFiltering,omitempty"` + // +kubebuilder:validation:Enum=802.1Q;802.1ad + VlanProtocol string `json:"vlanProtocol,omitempty"` } ``` @@ -355,67 +391,70 @@ _**Note 2**: multiple NICs can match the same policy on a host. In this case a s ```golang type SriovNetworkNodeStateSpec struct { - // ...existing fields... - Interfaces Interfaces `json:"interfaces,omitempty"` - Bridges Bridges `json:"bridges,omitempty"` + // ...existing fields... + Interfaces Interfaces `json:"interfaces,omitempty"` + Bridges Bridges `json:"bridges,omitempty"` } // SriovNetworkNodeStateStatus defines the observed state of SriovNetworkNodeState type SriovNetworkNodeStateStatus struct { - Interfaces InterfaceExts `json:"interfaces,omitempty"` - Bridges Bridges `json:"bridges,omitempty"` - SyncStatus string `json:"syncStatus,omitempty"` - LastSyncError string `json:"lastSyncError,omitempty"` + Interfaces InterfaceExts `json:"interfaces,omitempty"` + Bridges Bridges `json:"bridges,omitempty"` + SyncStatus string `json:"syncStatus,omitempty"` + LastSyncError string `json:"lastSyncError,omitempty"` } // Bridges contains list of bridges type Bridges struct { - OVS []OVSConfigExt `json:"ovs,omitempty"` - Linux []LinuxBridgeConfigExt `json:"linux,omitempty"` + OVS []OVSConfigExt `json:"ovs,omitempty"` + Linux []LinuxBridgeConfigExt `json:"linux,omitempty"` } type OVSConfigExt struct { - // name of the bridge - Name string `json:"name"` - // bridge-level configuration for the bridge - Bridge OVSBridgeConfig `json:"bridge,omitempty"` - // uplink-level bridge configuration for each uplink(PF). - // in the initial implementation will always contain one element - Uplinks []OVSUplinkConfigExt `json:"uplinks,omitempty"` + // name of the bridge + Name string `json:"name"` + // bridge-level configuration for the bridge + Bridge OVSBridgeConfig `json:"bridge,omitempty"` + // uplink-level bridge configuration for each uplink(PF). + // in the initial implementation will always contain one element + Uplinks []OVSUplinkConfigExt `json:"uplinks,omitempty"` } type OVSUplinkConfigExt struct { - // pci address of the PF - PciAddress string `json:"pciAddress"` - // name of the PF interface - Name string `json:"name,omitempty"` - // configuration from the Interface OVS table for the PF - Interface OVSInterfaceConfig `json:"interface,omitempty"` + // pci address of the PF + PciAddress string `json:"pciAddress"` + // name of the PF interface + Name string `json:"name,omitempty"` + // configuration from the Interface OVS table for the PF + Interface OVSInterfaceConfig `json:"interface,omitempty"` } type LinuxBridgeConfigExt struct { - Name string `json:"name"` - Bridge BridgeConfig `json:"bridge,omitempty"` - Uplinks []LinuxBridgeUPlinkConfigExt `json:"uplinks,omitempty"` + Name string `json:"name"` + Bridge BridgeConfig `json:"bridge,omitempty"` + Uplinks []LinuxBridgeUPlinkConfigExt `json:"uplinks,omitempty"` } type LinuxBridgeUPlinkConfigExt struct { - PciAddress string `json:"pciAddress"` - Name string `json:"name,omitempty"` - Uplink map[string]string `json:"uplink,omitempty"` + PciAddress string `json:"pciAddress"` + Name string `json:"name,omitempty"` + Uplink map[string]string `json:"uplink,omitempty"` } type VirtualFunction struct { - // ...existing fields... - // contains VF representor name for NICs in switchdev mode - RepresentorName string `json:"representorName,omitempty"` + // ...existing fields... + // contains VF representor name for NICs in switchdev mode + RepresentorName string `json:"representorName,omitempty"` } ``` -`SriovNetworkNodeState.spec` and `SriovNetworkNodeState.status` should be extended to contain the same `Bridges` struct. -_**Note:** The `Bridges` struct in the `SriovNetworkNodeState.status` can later be extended based on user feedback - to report additional information required to improve UX._ +`SriovNetworkNodeState.spec` and `SriovNetworkNodeState.status` should +be extended to contain the same `Bridges` struct. + +_**Note:** The `Bridges` struct in the `SriovNetworkNodeState.status` +can later be extended based on user feedback to report additional +information required to improve UX._ ### Implementation Details/Notes/Constraints @@ -423,13 +462,16 @@ The feature is only supported on baremetal clusters #### Dependencies on changes in other projects -The proposed implementation requires changes in `ovs-cni` and `accelerated-bridge-cni`. We need to change their behavior when `deviceID` argument is provided in CNI ARGS. -If `deviceID` is set and `bridge` arg is empty, the cni plugin should try to automatically select the right bridge by following the chain: - -VF (PCI address is in `deviceID` arg) > PF > Bond (if PF is part of the bond) > Bridge +The proposed implementation requires changes in `ovs-cni` and +`accelerated-bridge-cni`. We need to change their behavior when +`deviceID` argument is provided in CNI ARGS. If `deviceID` is set and +`bridge` arg is empty, the cni plugin should try to automatically +select the right bridge by following the chain: -_**Note:** `accelerated-bridge-cni` already has similar logic, but now it selects the bridge from the predefined list of bridges._ +VF (PCI address is in `deviceID` arg) > PF > Bond (if PF is part of the bond) > Bridge +_**Note:** `accelerated-bridge-cni` already has similar logic, but now +it selects the bridge from the predefined list of bridges._ #### Phased implementation @@ -440,7 +482,8 @@ The feature assumes phased implementation. Add support for [NIC configuration only flow](#nic-configuration-only-flow) and [Externally managed NIC flow](#externally-managed-nic-flow) for Open vSwitch -Requirements: +Requirements: + * add bridge auto-selection logic to `ovs-cni` * define `OVSNetwork` CRD and implement controller for it @@ -449,42 +492,49 @@ Requirements: Add support for [Fully automatic workflow](#fully-automatic-workflow) for Open vSwitch Requirements: + * requirements from phase 1 -* extend `SriovNetworkNodePolicy` and `SriovNetworkNodeState` CRDs to support configuration for ovs (bridge-level configuration) +* extend `SriovNetworkNodePolicy` and `SriovNetworkNodeState` CRDs to + support configuration for ovs (bridge-level configuration) * modify code: - * add support for ovs bridges creation - * add support for reporting information about configured ovs bridges on the node - * add support for removing auto-created ovs bridges during the PF reset + * add support for ovs bridges creation + * add support for reporting information about configured ovs bridges + on the node + * add support for removing auto-created ovs bridges during the PF reset ##### Phase 3 Add support for [NIC configuration only flow](#nic-configuration-only-flow), -[Externally managed NIC flow](#externally-managed-nic-flow) and [Fully automatic workflow](#fully-automatic-workflow) for Linux bridge +[Externally managed NIC flow](#externally-managed-nic-flow) and +[Fully automatic workflow](#fully-automatic-workflow) for Linux bridge Requirements: + * add bridge auto-selection logic to `accelerated-bridge-cni` * define `BridgeNetwork` CRD and implement controller for it -* extend `SriovNetworkNodePolicy` and `SriovNetworkNodeState` CRDs to support configuration for linux bridge (bridge-level configuration) +* extend `SriovNetworkNodePolicy` and `SriovNetworkNodeState` CRDs to + support configuration for linux bridge (bridge-level configuration) * modify code: - * add support for linux bridges creation - * add support for reporting information about configured linux bridges on the node - * add support for removing auto-created linux bridges during the PF reset + * add support for linux bridges creation + * add support for reporting information about configured linux bridges + on the node + * add support for removing auto-created linux bridges during the PF reset ### Upgrade & Downgrade considerations -This feature doesn't contain any breaking changes. +This feature doesn't contain any breaking changes. Automatic upgrades should be safe and will not require any manual steps. -Downgrading without PF configuration reset may be problematic and may keep the node in an inconsistent state. -It is recommended to reset PFs attached to bridges first and then do a downgrade. +Downgrading without PF configuration reset may be problematic and may +keep the node in an inconsistent state. It is recommended to reset PFs +attached to bridges first and then do a downgrade. ### Test Plan -New functionality should be covered with unit tests. - -Manual and automatic e2e testing will require hardware with NICs that support switchev mode and hardware offloading for software bridges. - +New functionality should be covered with unit tests. +Manual and automatic e2e testing will require hardware with NICs that +support switchev mode and hardware offloading for software bridges. ### Alternative options @@ -493,26 +543,37 @@ Manual and automatic e2e testing will require hardware with NICs that support sw ```golang type SriovNetworkPoolConfigSpec struct { - // OvsHardwareOffloadConfig describes the OVS HWOL configuration for selected Nodes - OvsHardwareOffloadConfig OvsHardwareOffloadConfig `json:"ovsHardwareOffloadConfig,omitempty"` - // NodeSelector only valid for the fields below - NodeSelector map[string]string `json:"nodeSelector,omitempty"` - Bridges []BridgeConf `json:"bridges,omitempty"` + // OvsHardwareOffloadConfig describes the OVS HWOL configuration for selected Nodes + OvsHardwareOffloadConfig OvsHardwareOffloadConfig `json:"ovsHardwareOffloadConfig,omitempty"` + // NodeSelector only valid for the fields below + NodeSelector map[string]string `json:"nodeSelector,omitempty"` + Bridges []BridgeConf `json:"bridges,omitempty"` } type BridgeConf struct { - // same configuration as in the main option - Bridge *Bridge `json:"bridge"` - // NicSelector uses the same type as SriovNetworkNodePolicySpec - NicSelector SriovNetworkNicSelector `json:"nicSelector"` + // same configuration as in the main option + Bridge *Bridge `json:"bridge"` + // NicSelector uses the same type as SriovNetworkNodePolicySpec + NicSelector SriovNetworkNicSelector `json:"nicSelector"` } ``` -The main problem with this option is that we can achieve the reliable scheduling of workloads only in a complicated way. -The scheduler considers information from the device plugins to ensure that required resources are available on the host before putting workloads on it. -In the main option outlined in this doc, a configuration of the bridge is a part of the `SriovNetworkNodePolicy` and that means that the bridge is for sure available on the host if the host announces resource name defined in `SriovNetworkNodePolicy`. - -In that alternative option, there is no warranty that NodeSelector + NicSelector for a configuration of bridges is in sync with the NodeSelector + NicSelectors from a policy, so we can't rely on the sriov resource name from the policy to do a reliable scheduling - host can have SRIOV VFs, but may miss a bridge. -To solve this problem, we can create a device plugin, which will expose information about available bridges in the form of resources, -e.g., `/_`. A user must explicitly request SRIOV + bridge resources while creating a Pod. \ No newline at end of file +The main problem with this option is that we can achieve the reliable +scheduling of workloads only in a complicated way. The scheduler +considers information from the device plugins to ensure that required +resources are available on the host before putting workloads on it. +In the main option outlined in this doc, a configuration of the bridge +is a part of the `SriovNetworkNodePolicy` and that means that the +bridge is for sure available on the host if the host announces resource +name defined in `SriovNetworkNodePolicy`. + +In that alternative option, there is no warranty that NodeSelector + +NicSelector for a configuration of bridges is in sync with the +NodeSelector + NicSelectors from a policy, so we can't rely on the +sriov resource name from the policy to do a reliable scheduling - host +can have SRIOV VFs, but may miss a bridge. To solve this problem, we +can create a device plugin, which will expose information about +available bridges in the form of resources, e.g., +`/_`. A user must explicitly +request SRIOV + bridge resources while creating a Pod. diff --git a/doc/design/switchdev-refactoring.md b/doc/design/switchdev-refactoring.md index ec0c0e93bb..fca2fb0cb8 100644 --- a/doc/design/switchdev-refactoring.md +++ b/doc/design/switchdev-refactoring.md @@ -7,44 +7,56 @@ creation-date: 14-12-2023 last-updated: 14-12-2023 --- -# switchdev and systemd modes refactoring +## switchdev and systemd modes refactoring -## Summary +### Summary -We need to refactor the implementation used for NICs in switchdev mode and align its behavior with the systemd -mode of the operator. The refactoring is required to simplify the development of the new switchdev-related -features for the sriov-network-operator. +We need to refactor the implementation used for NICs in switchdev mode +and align its behavior with the systemd mode of the operator. The +refactoring is required to simplify the development of the new +switchdev-related features for the sriov-network-operator. -## Motivation +### Motivation Currently the **sriov-network-operator** supports two configuration modes: + * `daemon` * `systemd` -The configuration mode can be changed by setting the `configurationMode` field in the `SriovOperatorConfig` CR. +The configuration mode can be changed by setting the +`configurationMode` field in the `SriovOperatorConfig` CR. _**Note**: This setting is global and applies to all sriov-network-operator-daemons in the cluster._ In the `daemon` mode, which historically is the first implemented mode, -the operator will setup NICs with _**SRIOV legacy**_ configuration directly in the **sriov-network-operator-daemon** -component by executing all enabled plugins. - -When the operator is in `systemd` mode, the **sriov-network-operator-daemon** component will execute most plugins -in the same way as in the `daemon` mode but will skip the call of the *generic* or *virtual* -plugin(when running in a virtualized environment) and instead will render config for the systemd -service that starts on the next OS boot and calls one of these plugins. -Then, the result of the service execution is handled by the **sriov-network-operator-daemon**. - -The `systemd` mode was implemented to support scenarios when, after the host reboot, -we need VFs to be configured before Kubernetes (kubelet) and Pods with workloads are started. - -To setup NICs with _**switchdev**_ configuration, the operator uses a different flow that ignores -the `configurationMode` setting. Two systemd services(not the same as used for systemd mode) are created on the node. -The first service is executed before the NetworkManager starts, and the second one after. -Both services run bash scripts. The script from the first service is responsible for VFs creation and for -switching a NIC to the switchdev eSwitch mode. The script from the second service binds VFs to the required drivers. - -If a NIC has _**switchdev**_ configuration, then `configurationMode` of the operator does not affect it. +the operator will setup NICs with _**SRIOV legacy**_ configuration +directly in the **sriov-network-operator-daemon** component by +executing all enabled plugins. + +When the operator is in `systemd` mode, the +**sriov-network-operator-daemon** component will execute most plugins +in the same way as in the `daemon` mode but will skip the call of the +_generic_ or _virtual_ plugin (when running in a virtualized +environment) and instead will render config for the systemd service +that starts on the next OS boot and calls one of these plugins. Then, +the result of the service execution is handled by the +**sriov-network-operator-daemon**. + +The `systemd` mode was implemented to support scenarios when, after +the host reboot, we need VFs to be configured before Kubernetes +(kubelet) and Pods with workloads are started. + +To setup NICs with _**switchdev**_ configuration, the operator uses a +different flow that ignores the `configurationMode` setting. Two +systemd services (not the same as used for systemd mode) are created +on the node. The first service is executed before the NetworkManager +starts, and the second one after. Both services run bash scripts. The +script from the first service is responsible for VFs creation and for +switching a NIC to the switchdev eSwitch mode. The script from the +second service binds VFs to the required drivers. + +If a NIC has _**switchdev**_ configuration, then `configurationMode` +of the operator does not affect it. ```mermaid --- @@ -69,52 +81,63 @@ flowchart TD; #### Problems of the current implementation -* it is confusing that `configurationMode` does not affect devices with switchdev configuration. -* system services for switchdev configuration are shell scripts completely independent from the -main code base and it is hard to extend them with new functionality. -* for switchdev NICs, VF configuration flow (bash-based) has some limitations compared to legacy VF configuration. -* it is impossible to apply switchdev configuration for the NIC without reboot. - -### Use Cases - -* As a developer I don't want to maintain the code with similar logic in two places: -switchdev bash scrips and **sriov-network-operator-daemon** code. -* As a developer and a user I want to have only one set of systemd services that handle -both _**switchdev**_ configurations and `systemd` mode. -* As a user I want `configurationMode` to work the same way for NICs with -_**legacy**_ and _**switchdev**_ configurations. -* As a user I want to apply _**switchdev**_ configuration for NIC by the **sriov-network-operator-daemon** -without reboot (in case if reboot is not required by other logic, -e.g. kernel parameters configuration, FW configuration). - -### Goals - -* it should be possible to apply _**switchdev**_ configuration in the **sriov-network-operator-daemon** without reboot. -* the code used by `daemon` and `systemd` modes to handle _**switchdev**_ and _**legacy**_ -configurations should be the same Golang code. -* `configurationMode` option should work the same for NICs with _**legacy**_ and _**switchdev**_ configurations. -* the operator should use unified systemd services which will be deployed only -if the operator works in the `systemd` mode. -* `systemd` mode should be changed to support 2 stage configuration: -pre system network manager (NetworkManager or netplan) and after system network manager. - _Note: This is required to support all use-cases supported by the current switchdev implementation._ - - -### Non-Goals +* it is confusing that `configurationMode` does not affect devices + with switchdev configuration. +* system services for switchdev configuration are shell scripts + completely independent from the main code base and it is hard to + extend them with new functionality. +* for switchdev NICs, VF configuration flow (bash-based) has some + limitations compared to legacy VF configuration. +* it is impossible to apply switchdev configuration for the NIC + without reboot. + +#### Use Cases + +* As a developer I don't want to maintain the code with similar logic + in two places: switchdev bash scrips and + **sriov-network-operator-daemon** code. +* As a developer and a user I want to have only one set of systemd + services that handle both _**switchdev**_ configurations and + `systemd` mode. +* As a user I want `configurationMode` to work the same way for NICs + with _**legacy**_ and _**switchdev**_ configurations. +* As a user I want to apply _**switchdev**_ configuration for NIC by + the **sriov-network-operator-daemon** without reboot (in case if + reboot is not required by other logic, e.g. kernel parameters + configuration, FW configuration). + +#### Goals + +* it should be possible to apply _**switchdev**_ configuration in the + **sriov-network-operator-daemon** without reboot. +* the code used by `daemon` and `systemd` modes to handle + _**switchdev**_ and _**legacy**_ configurations should be the same + Golang code. +* `configurationMode` option should work the same for NICs with + _**legacy**_ and _**switchdev**_ configurations. +* the operator should use unified systemd services which will be + deployed only if the operator works in the `systemd` mode. +* `systemd` mode should be changed to support 2 stage configuration: + pre system network manager (NetworkManager or netplan) and after + system network manager. + _Note: This is required to support all use-cases supported by the + current switchdev implementation._ + +#### Non-Goals * replace _Externally Manage PF_ feature * remove all shell scripts from the code -## Proposal +### Proposal 1. Drop existing bash-based implementation which is used for NICs with _**switchdev**_ configuration 2. Modify _generic_ and _virtual (if required)_ plugins to support _**switchdev**_ configuration -3. Modify code related to the _Externally Manage PF_ feature +3. Modify code related to the _Externally Manage PF_ feature to support _**switchdev**_ configuration 4. Modify `systemd` mode flow to handle devices with both _**legacy**_ and _**switchdev**_ configurations 5. Split `systemd` mode system service to two parts: - - `pre` - executes before NetworkManager/netplan and OVS - - `after` - executes after NetworkManager/netplan and OVS + * `pre` - executes before NetworkManager/netplan and OVS + * `after` - executes after NetworkManager/netplan and OVS ```mermaid --- @@ -132,75 +155,84 @@ flowchart TD; ``` +#### Workflow Description -### Workflow Description - -Users using only NICs with _**legacy**_ SRIOV configurations will not need to change their workflow. -The operator should behave for these configurations the same way as it does now. +Users using only NICs with _**legacy**_ SRIOV configurations will not +need to change their workflow. The operator should behave for these +configurations the same way as it does now. -Users using NICs with _**switchdev**_ configurations will need to explicitly set operator's -`configurationMode` to `systemd` if they expect the configuration of the NIC to happen -early on boot (before Kubernetes starts) to support the hwoffload use-case. +Users using NICs with _**switchdev**_ configurations will need to +explicitly set operator's `configurationMode` to `systemd` if they +expect the configuration of the NIC to happen early on boot (before +Kubernetes starts) to support the hwoffload use-case. -### API Extensions +#### API Extensions -#### SriovNetworkNodeState CR +##### SriovNetworkNodeState CR `SriovNetworkNodeState.status.interfaces[*].Vfs[*].vdpaType` field should be added. -This field should be used to report information about type of the VDPA +This field should be used to report information about type of the VDPA device that is configured for VF. -Empty string means that there is no VDPA device. +Empty string means that there is no VDPA device. Valid values are: `virtio`, `vhost` (same as in `SriovNetworkNodePolicySpec`) -``` +```go type VirtualFunction struct { - Name string `json:"name,omitempty"` - Mac string `json:"mac,omitempty"` - Assigned string `json:"assigned,omitempty"` - Driver string `json:"driver,omitempty"` - PciAddress string `json:"pciAddress"` - Vendor string `json:"vendor,omitempty"` - DeviceID string `json:"deviceID,omitempty"` - Vlan int `json:"Vlan,omitempty"` - Mtu int `json:"mtu,omitempty"` - VfID int `json:"vfID"` -+ VdpaType string `json:"vdpaType,omitempty"` + Name string `json:"name,omitempty"` + Mac string `json:"mac,omitempty"` + Assigned string `json:"assigned,omitempty"` + Driver string `json:"driver,omitempty"` + PciAddress string `json:"pciAddress"` + Vendor string `json:"vendor,omitempty"` + DeviceID string `json:"deviceID,omitempty"` + Vlan int `json:"Vlan,omitempty"` + Mtu int `json:"mtu,omitempty"` + VfID int `json:"vfID"` ++ VdpaType string `json:"vdpaType,omitempty"` } ``` -#### SriovOperatorConfig CR +##### SriovOperatorConfig CR Change in the operator's behavior: `configurationMode` option now have effect on NICs with _**switchdev**_ configurations. -### Implementation Details/Notes/Constraints +#### Implementation Details/Notes/Constraints We should consider improving unit-test coverage for modified code parts during the implementation. -After the operator upgrade, we should clean up from the host unneeded files (scripts, system services, config files) created by the previous version of the operator. +After the operator upgrade, we should clean up from the host unneeded +files (scripts, system services, config files) created by the previous +version of the operator. -### Upgrade & Downgrade considerations +#### Upgrade & Downgrade considerations -* after upgrading the operator, _**switchdev**_ config will be applied by **sriov-network-operator-daemon** and not by systemd service unless the user changes `configurationMode` setting to `systemd` -* after upgrading the operator, "implicit mixed mode" when _**switchdev**_ NIC configurations are handled by bash scripts(in systemd services) -and _**legacy**_ NIC configurations are managed by **sriov-network-operator-daemon** will not be supported anymore. +* after upgrading the operator, _**switchdev**_ config will be applied + by **sriov-network-operator-daemon** and not by systemd service + unless the user changes `configurationMode` setting to `systemd` +* after upgrading the operator, "implicit mixed mode" when + _**switchdev**_ NIC configurations are handled by bash scripts (in + systemd services) and _**legacy**_ NIC configurations are managed by + **sriov-network-operator-daemon** will not be supported anymore. -_Note: `configurationMode` is a global setting, so the user will need to decide -which mode to use for the entire cluster_ +_Note: `configurationMode` is a global setting, so the user will need +to decide which mode to use for the entire cluster_ -Upgrade/Downgrade for users using only NICs with _**legacy**_ configurations will not require any actions. -Upgrade/Downgrade for clusters with _**switchdev**_ configurations will require -changing the operator's `configurationMode` option. +Upgrade/Downgrade for users using only NICs with _**legacy**_ +configurations will not require any actions. Upgrade/Downgrade for +clusters with _**switchdev**_ configurations will require changing the +operator's `configurationMode` option. -### Test Plan +#### Test Plan The proposed changes will not introduce new functionality. -After the refactoring, _**switchdev**_ configurations will also be supported in the `daemon` mode. -This is the only thing we may need to develop additional tests for. -All other changes should be validated by running regression testing. +After the refactoring, _**switchdev**_ configurations will also be +supported in the `daemon` mode. This is the only thing we may need to +develop additional tests for. All other changes should be validated by +running regression testing. -_Note: behavior for _**switchdev**_ configurations will be changed in a non-fully compatible way; -this may require to fix existing tests._ +_Note: behavior for _**switchdev**_ configurations will be changed in +a non-fully compatible way; this may require to fix existing tests._ diff --git a/doc/hacking.md b/doc/hacking.md index 2929d4fd9f..d37865e9cd 100644 --- a/doc/hacking.md +++ b/doc/hacking.md @@ -1,6 +1,9 @@ # Hacking Guide -The sriov network operator relies on operator-sdk and kubebuilder to scaffold and generate code and manifests. We keeps upgrading sdk version for the operator. Now the operator is compliance with operator-sdk 1.9.0 and go.kubebuilder.io/v3. +The sriov network operator relies on operator-sdk and kubebuilder to scaffold +and generate code and manifests. We keeps upgrading sdk version for the operator. +Now the operator is compliance with operator-sdk 1.9.0 and +go.kubebuilder.io/v3. ## Build and Test @@ -30,9 +33,12 @@ make plugins ``` If you want to test changes to the `network config daemon`, you must: -- build and tag an image locally with `docker build -f Dockerfile.sriov-network-config-daemon -t imagename` + +- build and tag an image locally with + `docker build -f Dockerfile.sriov-network-config-daemon -t imagename` - push the image to a registry -- change `hack/env.sh` value for `SRIOV_NETWORK_CONFIG_DAEMON_IMAGE` pointing _imagename_ from the registry you pushed the image to +- change `hack/env.sh` value for `SRIOV_NETWORK_CONFIG_DAEMON_IMAGE` pointing + _imagename_ from the registry you pushed the image to and then `make run` @@ -45,6 +51,7 @@ Refer to the operator-sdk's [instruction](https://sdk.operatorframework.io/docs/ 1. Edit the *_types.go file to change the definitions for the Spec and Status of the Kinds. 2. Generate Go code, CRD. + ```bash # Generate controller Go code make generate @@ -60,9 +67,13 @@ Refer to the operator-sdk's [instruction](https://sdk.operatorframework.io/docs/ ## Upgrading operator-sdk -To upgrade the generated code to a new operator-sdk version, we need to follow the instructions in [operator-sdk's migration guide](https://sdk.operatorframework.io/docs/upgrading-sdk-version/). +To upgrade the generated code to a new operator-sdk version, we need to follow +the instructions in the +[operator-sdk's migration guide](https://sdk.operatorframework.io/docs/upgrading-sdk-version/). -In addition, we must ensure that the k8s dependencies in the operator's go.mod match the selected version of operator-sdk. For example, for operator-sdk v0.19.x, check the k8s dependencies: +In addition, we must ensure that the k8s dependencies in the operator's go.mod +match the selected version of operator-sdk. For example, for operator-sdk +v0.19.x, check the k8s dependencies: Identify kubebuilder version referenced by operator-sdk @@ -70,41 +81,47 @@ Identify controller-runtime version referenced by kubebuilder Check controller-runtime's go.mod file -As a result, we can determine the versions of the k8s dependencies in the operator's go.mod. +As a result, we can determine the versions of the k8s dependencies in the +operator's go.mod. ## Build an custom image To build the SR-IOV network operator container image: - ```bash - make image +```bash +make image +``` -If you want to build another image (e.g. webhook or config-daemon), you'll need to do -the following: +If you want to build another image (e.g. webhook or config-daemon), you'll need +to do the following: - ```bash - export DOCKERFILE=Dockerfile.sriov-network-config-daemon - export APP_NAME=sriov-network-config-daemon - make image +```bash +export DOCKERFILE=Dockerfile.sriov-network-config-daemon +export APP_NAME=sriov-network-config-daemon +make image - export DOCKERFILE=Dockerfile.webhook - export APP_NAME=sriov-network-webhook - make image +export DOCKERFILE=Dockerfile.webhook +export APP_NAME=sriov-network-webhook +make image +``` Then you'll need to push the image to a registry using e.g. `buildah push`. -Before deploying the Operator, you want to export these variables to use that custom image: +Before deploying the Operator, you want to export these variables to use that +custom image: - ```bash - export SRIOV_NETWORK_CONFIG_DAEMON_IMAGE= - (...) +```bash +export SRIOV_NETWORK_CONFIG_DAEMON_IMAGE= +(...) +``` ## Enable Unsupported NICs By default, unsupported NICs are not reported in `SriovNetworkNodeState` and are not allowed in `SriovNetworkNodePolicy` by the webhook. -If you want to allow unsupported NICs, set the `DEV_MODE` env var to `TRUE`. +If you want to allow unsupported NICs, set the `DEV_MODE` env var to +`TRUE`. ```bash export DEV_MODE=TRUE -``` \ No newline at end of file +``` diff --git a/doc/ib-vf-guid-static-configuration.md b/doc/ib-vf-guid-static-configuration.md index f0ae51fe21..99b591adbe 100644 --- a/doc/ib-vf-guid-static-configuration.md +++ b/doc/ib-vf-guid-static-configuration.md @@ -1,6 +1,7 @@ # Infiniband VF GUID Static Configuration -SR-IOV Network Operator is able to use a static configuration file from the host filesystem to assign GUIDs to IB VFs. +SR-IOV Network Operator is able to use a static configuration file from the +host filesystem to assign GUIDs to IB VFs. ## Prerequisites @@ -9,9 +10,11 @@ SR-IOV Network Operator is able to use a static configuration file from the host ### Configuration file -Config file should be stored at `/etc/sriov-operator/infiniband/guids`. This location is writable across most cloud platforms. +Config file should be stored at `/etc/sriov-operator/infiniband/guids`. This +location is writable across most cloud platforms. -VF to GUID assignment, based on this file, is ordered. VF0 takes the GUID0, VF1 takes the GUID1 etc. +VF to GUID assignment, based on this file, is ordered. VF0 takes the GUID0, +VF1 takes the GUID1 etc. Each PF has its own set of GUIDs. @@ -38,20 +41,24 @@ Example of the config file: Config file parameters: -* `pci_address` is a PCI address of a PF -* `pf_guid` is a GUID of a PF. Can be obtained with `ibstat` command. -* `guids` is an array of VF GUID strings -* `guidsRange` is an object representing the start and end of a VF GUID range. It has two fields: - * `start` is a VF GUID range start - * `end` is a VF GUID range end +- `pci_address` is a PCI address of a PF +- `pf_guid` is a GUID of a PF. Can be obtained with `ibstat` command. +- `guids` is an array of VF GUID strings +- `guidsRange` is an object representing the start and end of a VF GUID range. + It has two fields: + - `start` is a VF GUID range start + - `end` is a VF GUID range end Requirements for the config file: -* `pci_address` and `pf_guid` cannot be set at the same time for a single device - should return an error -* if the list contains multiple entries for the same device, the first one shall be taken -* `rangeStart` and `rangeEnd` are both included in the range -* `guids` list and range cannot be both set at the same time for a single device - should return an error -* GUIDs are assigned once and not change throughout the lifecycle of the host +- `pci_address` and `pf_guid` cannot be set at the same time for a single + device - should return an error +- if the list contains multiple entries for the same device, the first one + shall be taken +- `rangeStart` and `rangeEnd` are both included in the range +- `guids` list and range cannot be both set at the same time for a single + device - should return an error +- GUIDs are assigned once and not change throughout the lifecycle of the host ### Deploy SriovNetworkNodePolicy @@ -87,4 +94,3 @@ ip link vf 0 link/infiniband ... NODE_GUID 02:00:00:00:00:00:00:00, PORT_GUID 02:00:00:00:00:00:00:00, link-state enable, trust off, query_rss off vf 1 link/infiniband ... NODE_GUID 02:00:00:00:00:00:00:01, PORT_GUID 02:00:00:00:00:00:00:01, link-state enable, trust off, query_rss off ``` - diff --git a/doc/ovs-hw-offload.md b/doc/ovs-hw-offload.md index 4e287b8116..8af7e3f001 100644 --- a/doc/ovs-hw-offload.md +++ b/doc/ovs-hw-offload.md @@ -5,13 +5,15 @@ and preventing fully utilizing available bandwidth. OVS 2.8 and above support a feature called OVS Hardware Offload which improves performance significantly. This feature allows offloading the OVS data-plane to the NIC while maintaining OVS control-plane unmodified. It is using SR-IOV technology with VF representor -host net-device. The VF representor plays the same role as TAP devices -in Para-Virtual (PV) setup. A packet sent through the VF representor on the host -arrives to the VF, and a packet sent through the VF is received by its representor. +host net-device. The VF representor plays the same role as TAP devices in +Para-Virtual (PV) setup. A packet sent through the VF representor on the host +arrives to the VF, and a packet sent through the VF is received by its +representor. OVS Hardware Offloading requires NIC to be configured in `switchdev` mode. -The operator can automate the creation and configuration of OVS bridges when the "manageSoftwareBridges" featureGate is activated. +The operator can automate the creation and configuration of OVS bridges when +the "manageSoftwareBridges" featureGate is activated. ## Supported Ethernet controllers @@ -25,22 +27,23 @@ The following manufacturers are known to work: ### Activate "manageSoftwareBridges" featureGate -``` +```bash kubectl patch sriovoperatorconfigs.sriovnetwork.openshift.io -n network-operator default --patch '{ "spec": { "featureGates": { "manageSoftwareBridges": true } } }' --type='merge' ``` - ### Deploy SriovNetworkNodePolicy -The example policy below selects all NVIDIA ConnectX-6 Dx devices on all worker nodes. +The example policy below selects all NVIDIA ConnectX-6 Dx devices on all +worker nodes. The following actions will apply to selected NICs: -* set NIC's eswitch mode to `switchdev` +- set NIC's eswitch mode to `switchdev` -* create 5 Virtual Functions on each Physical Function +- create 5 Virtual Functions on each Physical Function -* create a separate OVS bridge (with default configuration that is suitable for HW-offloading with OVS-kernel dataplane) for each Physical Function (PF) +- create a separate OVS bridge (with default configuration that is suitable for + HW-offloading with OVS-kernel dataplane) for each Physical Function (PF) ```yaml apiVersion: sriovnetwork.openshift.io/v1 @@ -62,8 +65,10 @@ spec: ovs: {} ``` -_Note: `spec.bridge.ovs: {}` - means use default settings (suitable for HW-offloading with OVS-kernel dataplane) -The fields defined in the [Bridge type](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/api/v1/sriovnetworknodepolicy_types.go#L84) can be used to configure advanced bridge and interface level options._ +_Note: `spec.bridge.ovs: {}` - means use default settings (suitable for +HW-offloading with OVS-kernel dataplane). The fields defined in the +[Bridge type](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/api/v1/sriovnetworknodepolicy_types.go#L84) +can be used to configure advanced bridge and interface level options._ The spec above will render to the similar SriovNetworkNodeState for matching nodes. @@ -113,17 +118,18 @@ spec: vfRange: 0-4 ``` -In this example node-a has single ConnectX-6 Dx card with two ports (two Physical Functions). -For each Physical Function a separate OVS bridge will be created. +In this example node-a has single ConnectX-6 Dx card with two ports (two +Physical Functions). For each Physical Function a separate OVS bridge will be +created. PF `0000:d8:00.0` -> OVS-bridge `br-0000_d8_00.0` PF `0000:d8:00.1` -> OVS-bridge `br-0000_d8_00.1` -The PCI address of the PF is used to generate a predictable name for the bridge. +The PCI address of the PF is used to generate a predictable name for the +bridge. The PFs will be automatically attached to the bridges. - ### Create kind: OVSNetwork CR ```yaml @@ -149,7 +155,9 @@ spec: vlan: 200 ``` -_Note: There is no need to explicitly specify bridge name in the OVSNetwork. The `ovs-cni` will be able to automatically select the right OVS bridge based on the allocated VF for the Pod._ +_Note: There is no need to explicitly specify bridge name in the OVSNetwork. +The `ovs-cni` will be able to automatically select the right OVS bridge based +on the allocated VF for the Pod._ ### Deploy POD with OVS hardware-offload @@ -193,7 +201,8 @@ Run iperf3 client on POD 2 kubectl exec -it ovs-offload-pod2 -- iperf3 -c 192.168.1.17 -t 100 ``` -Check traffic on the VF representor port. Verify only TCP connection establishment appears +Check traffic on the VF representor port. Verify only TCP connection +establishment appears ```text tcpdump -i enp3s0f0_3 tcp diff --git a/doc/quickstart.md b/doc/quickstart.md index 18d6867418..cdce934ce7 100644 --- a/doc/quickstart.md +++ b/doc/quickstart.md @@ -2,14 +2,21 @@ ## Prerequisites -1. A supported SRIOV hardware on the cluster nodes. Supported models can be found [here](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/supported-hardware.md). +1. A supported SRIOV hardware on the cluster nodes. Supported models can be + found in the + [supported hardware list](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/supported-hardware.md). 2. Kubernetes or Openshift cluster running on bare metal nodes. -3. Multus-cni is deployed as default CNI plugin, and there is a default CNI plugin (flannel, openshift-sdn etc.) available for Multus-cni. -4. On RedHat Enterprise Linux and Ubuntu operating systems, the `rdma-core` package must be installed to support RDMA resource provisioning. On RedHat CoreOS the package installation is not required. +3. Multus-cni is deployed as default CNI plugin, and there is a default CNI + plugin (flannel, openshift-sdn etc.) available for Multus-cni. +4. On RedHat Enterprise Linux and Ubuntu operating systems, the `rdma-core` + package must be installed to support RDMA resource provisioning. On RedHat + CoreOS the package installation is not required. ## Installation -Make sure to have installed the Operator-SDK, as shown in its [install documentation](https://sdk.operatorframework.io/docs/installation/), and that the binaries are available in your \$PATH. +Make sure to have installed the Operator-SDK, as shown in its +[install documentation](https://sdk.operatorframework.io/docs/installation/), +and that the binaries are available in your \$PATH. Clone this GitHub repository. @@ -26,15 +33,20 @@ make deploy-setup ``` If you are running a Kubernetes cluster: + ```bash make deploy-setup-k8s ``` -Webhooks are disabled when deploying on a Kubernetes cluster as per the instructions above. To enable webhooks on Kubernetes cluster, there are two options: +Webhooks are disabled when deploying on a Kubernetes cluster as per the +instructions above. To enable webhooks on Kubernetes cluster, there are two +options: -1. Create certificates for each of the two webhooks using a single CA whose cert you provide through an environment variable. +1. Create certificates for each of the two webhooks using a single CA whose + cert you provide through an environment variable. For example, given `cacert.pem`, `key.pem` and `cert.pem`: + ```bash kubectl create ns sriov-network-operator kubectl -n sriov-network-operator create secret tls operator-webhook-cert --cert=cert.pem --key=key.pem @@ -45,12 +57,14 @@ Webhooks are disabled when deploying on a Kubernetes cluster as per the instruct make deploy-setup-k8s ``` -2. Using https://cert-manager.io/, deploy it as: +2. Using , deploy it as: + ```bash kubectl apply -f https://github.com/jetstack/cert-manager/releases/download/v1.3.0/cert-manager.yaml ``` Define the appropriate Issuer and Certificates, as an example: + ```bash kubectl create ns sriov-network-operator cat < **Note:** sriov-network-operator maintains a list of supported NICs which it supports. -> These are stored in supported-nic-ids [configMap](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deployment/sriov-network-operator/templates/configmap.yaml). -> The operator uses this list to enforce it only operates on NICs that are supported. For unsupported SR-IOV NICs, that is not guaranteed, but might work as well. -> To have sriov-network-operator operate on an unsupported NIC, after installing the operator, you have to add the unsupported SR-IOV NICs information to the ConfigMap -> in following format: `: `. +> **Note:** sriov-network-operator maintains a list of supported NICs which it +> supports. These are stored in the supported-nic-ids +> [configMap](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deployment/sriov-network-operator/templates/configmap.yaml). +> The operator uses this list to enforce it only operates on NICs that are +> supported. For unsupported SR-IOV NICs, that is not guaranteed, but might +> work as well. To have sriov-network-operator operate on an unsupported NIC, +> after installing the operator, you have to add the unsupported SR-IOV NICs +> information to the ConfigMap in following format: +> `: `. > Then restart the config daemon and operator webhook pods. ## Supported features per hardware The following table depicts the supported SR-IOV hardware features of each supported hardware: -| Model | SR-IOV Kernel | SR-IOV DPDK | SR-IOV Hardware Offload (switchdev) | -| ------------------------ | ------------- | ----------- |------------------------------------ | +| Model | SR-IOV Kernel | SR-IOV DPDK | SR-IOV Hardware Offload (switchdev) | +| ----- | ------------- | ----------- | ----------------------------------- | | Intel XXV710 Family | V | V | X | | Intel X710 10GbE Backplane | V | V | X | -| Intel X710 10GbE Base T | V | V | X | +| Intel X710 10GbE Base T | V | V | X | | Intel X550 Family | V | V | X | | Intel X557 Family | V | V | X | | Intel E810 Family | V | V | X | @@ -81,39 +85,56 @@ The following table depicts the supported SR-IOV hardware features of each suppo | Red_Hat_Virtio_network_device | X | V | X | | Red_Hat_Virtio_1_0_network_device | X | V | X | -# Adding new Hardware +## Adding new Hardware ## Initial support -Vendors (or other parties) interested in adding a new supported hardware to the supported list of devices of sriov-network-operator -should follow the following procedure: + +Vendors (or other parties) interested in adding a new supported hardware to +the supported list of devices of sriov-network-operator should follow the +following procedure: * Open an Issue requesting to add support for new hardware * Specify which hardware it is (Model, Vendor ID, Device ID) * Perform Testing on requested hardware using: * Kubernetes last release * Either last release version or master version of sriov-network-operator - * Note: We do not have a specifc list of test-cases however you should at a minimum ensure - sriov-network-operator is able to properly discover your device, configure SR-IOV and - expose them as a kubernetes node resource, and you are able to run workloads consuming - those resources. + * Note: We do not have a specifc list of test-cases however you should at a + minimum ensure sriov-network-operator is able to properly discover your + device, configure SR-IOV and expose them as a kubernetes node resource, + and you are able to run workloads consuming those resources. * Add information of what was tested to the issue opened -* Add contact point information to [vendor-support.md](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/vendor-support.md), so we know who to reach out if issues arise when running sriov-network-operator against the specified hardware. -* Submit PR to add your device to this file as well as to supported-nic-ids configMap [here](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deployment/sriov-network-operator/templates/configmap.yaml) and [here](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deploy/configmap.yaml). +* Add contact point information to the + [vendor-support.md](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/vendor-support.md), + so we know who to reach out if issues arise when running + sriov-network-operator against the specified hardware. +* Submit PR to add your device to this file as well as to the + supported-nic-ids configMap in the + [deployment configmap](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deployment/sriov-network-operator/templates/configmap.yaml) + and the + [deploy configmap](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/deploy/configmap.yaml). * The tables above should be updated according to what was tested ## Continuous support -To ensure sriov-network-operator continues to operate as expected on supported hardware it is recommended that hardware vendors (or another party) -adds CI which runs against PRs in the project. Without it we cannot commit for sriov-network-operator to continue to work properly on the specified -hardware. -Additional information on how to add Vendor/3rd-party CI can be found [here](https://github.com/k8snetworkplumbingwg/sriov-network-operator/tree/master/ci). +To ensure sriov-network-operator continues to operate as expected on supported +hardware it is recommended that hardware vendors (or another party) adds CI +which runs against PRs in the project. Without it we cannot commit for +sriov-network-operator to continue to work properly on the specified hardware. + +Additional information on how to add Vendor/3rd-party CI can be found in the +[ci directory](https://github.com/k8snetworkplumbingwg/sriov-network-operator/tree/master/ci). ### E2E tests -sriov-network-operator offers a set of e2e tests vendors can run on their hardware. These tests utilize [Kind](https://kind.sigs.k8s.io/) to spin up -a Kubernetes cluster and run tests. Information on how to run e2e tests can be found [here](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/testing-kind.md). -These tests may be used as part of vendor CIs added to the project to validate sriov-network-operator is able to operate -on the relevant hardware. +sriov-network-operator offers a set of e2e tests vendors can run on their +hardware. These tests utilize [Kind](https://kind.sigs.k8s.io/) to spin up a +Kubernetes cluster and run tests. Information on how to run e2e tests can be +found in the +[testing-kind guide](https://github.com/k8snetworkplumbingwg/sriov-network-operator/blob/master/doc/testing-kind.md). +These tests may be used as part of vendor CIs added to the project to validate +sriov-network-operator is able to operate on the relevant hardware. ->**Note:** The maintainers of this project reserve the right to remove a device from the supported list if issues arise on that hardware. -> this will be done only after attempting to contact the contact point provided by a specific vendor to address the issues. +>**Note:** The maintainers of this project reserve the right to remove a +> device from the supported list if issues arise on that hardware. This will +> be done only after attempting to contact the contact point provided by a +> specific vendor to address the issues. diff --git a/doc/testing-kind.md b/doc/testing-kind.md index f159ccaf83..7811ee3c1b 100644 --- a/doc/testing-kind.md +++ b/doc/testing-kind.md @@ -1,73 +1,96 @@ -## E2E test with KinD -Kubernetes IN Docker (KIND) is a tool to deploy Kubernetes inside Docker containers. It is used to test multi nodes scenarios on a single baremetal node. -To run the E2E tests inside a KIND cluster, `./hack/run-e2e-test-kind.sh` can be used. The script performs the following operations: +# E2E test with KinD - * Deploys a 2 node KIND cluster (master and worker) - * Moves the specified SR-IOV capable PCI net device to KIND worker namespace - * Deploys the operator - * Runs E2E tests +Kubernetes IN Docker (KIND) is a tool to deploy Kubernetes inside Docker +containers. It is used to test multi nodes scenarios on a single baremetal +node. To run the E2E tests inside a KIND cluster, +`./hack/run-e2e-test-kind.sh` can be used. The script performs the following +operations: -There are two modes of moving the specified SR-IOV capable PCI net device to the KIND worker namespace: +* Deploys a 2 node KIND cluster (master and worker) +* Moves the specified SR-IOV capable PCI net device to KIND worker namespace +* Deploys the operator +* Runs E2E tests - * `test-suite` (default): In this mode, the E2E test suite handle the PF and its VFs switching to the test namespace. - * `system-service` mode: In this mode a dedicated system service is used to switch the PF and VFs to the test namespace. +There are two modes of moving the specified SR-IOV capable PCI net device to +the KIND worker namespace: -The mode can be selected using the `INTERFACES_SWITCHER` environment variable, or by passing the mode to the `./hack/run-e2e-test-kind.sh` script using the `--device-netns-switcher` flag. +* `test-suite` (default): In this mode, the E2E test suite handle the PF and + its VFs switching to the test namespace. +* `system-service` mode: In this mode a dedicated system service is used to + switch the PF and VFs to the test namespace. -### How to test -To execute E2E tests, a SR-IOV Physical Function device is required and will be added to a KinD workers network namespace, depending on the device netns switcher method, the testing steps can defer. +The mode can be selected using the `INTERFACES_SWITCHER` environment variable, +or by passing the mode to the `./hack/run-e2e-test-kind.sh` script using the +`--device-netns-switcher` flag. + +## How to test + +To execute E2E tests, a SR-IOV Physical Function device is required and will +be added to a KinD workers network namespace, depending on the device netns +switcher method, the testing steps can defer. Note: Test device will remain in KinD worker node until cluster is terminated. -#### Device netns switcher mode `test-suite` -``` -$ git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git -$ cd sriov-network-operator/ -$ source hack/get-e2e-kind-tools.sh -$ export TEST_PCI_DEVICE=0000:02:00.0 -$ sudo ./hack/run-e2e-test-kind.sh --pf-pci-address $TEST_PCI_DEVICE -``` +### Device netns switcher mode `test-suite` -#### Device netns switcher mode `system-service` -The `system-service` mode uses a linux service to handle the interface switching. To prepare the service, the following needs to be done as root: +```bash +git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git +cd sriov-network-operator/ +source hack/get-e2e-kind-tools.sh +export TEST_PCI_DEVICE=0000:02:00.0 +sudo ./hack/run-e2e-test-kind.sh --pf-pci-address $TEST_PCI_DEVICE ``` + +### Device netns switcher mode `system-service` + +The `system-service` mode uses a linux service to handle the interface +switching. To prepare the service, the following needs to be done as root: + +```bash cp ./hack/vf-netns-switcher.sh /usr/bin/ cp ./hack/vf-switcher.service /etc/systemd/system/ systemctl daemon-reload ``` + For the service to work properly the `jq` tool is needed. To run the E2E tests do: -``` -$ git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git -$ cd sriov-network-operator/ -$ source hack/get-e2e-kind-tools.sh -$ KUBECONFIG=/etc/kubernetes/admin.conf -$ INTERFACES_SWITCHER=system-service -$ ./hack/run-e2e-test-kind.sh --pf-pci-address -``` -### How to repeat test using existing KinD cluster -Export test PCI device used to set up KinD cluster and export KinD worker network namespace path: -``` -$ export KUBECONFIG="${HOME}/.kube/config" -$ export TEST_PCI_DEVICE=0000:02:00.0 -$ export TEST_NETNS_PATH=$(docker inspect "$(docker ps --filter 'name=kind-worker' -q)" --format "{{ .NetworkSettings.SandboxKey }}") -$ sudo make test-e2e-k8s +```bash +git clone https://github.com/k8snetworkplumbingwg/sriov-network-operator.git +cd sriov-network-operator/ +source hack/get-e2e-kind-tools.sh +KUBECONFIG=/etc/kubernetes/admin.conf +INTERFACES_SWITCHER=system-service +./hack/run-e2e-test-kind.sh --pf-pci-address ``` -### How to teardown +## How to repeat test using existing KinD cluster -``` -$ ./hack/teardown-e2e-kind-cluster.sh +Export test PCI device used to set up KinD cluster and export KinD worker +network namespace path: + +```bash +export KUBECONFIG="${HOME}/.kube/config" +export TEST_PCI_DEVICE=0000:02:00.0 +export TEST_NETNS_PATH=$(docker inspect "$(docker ps --filter 'name=kind-worker' -q)" --format "{{ .NetworkSettings.SandboxKey }}") +sudo make test-e2e-k8s ``` -#### Cleaning up the `system-service` service files +## How to teardown + +```bash +./hack/teardown-e2e-kind-cluster.sh ``` -$ sudo rm -f /etc/systemd/system/vf-switcher.service -$ sudo rm -f /usr/bin/vf-netns-switcher.sh -$ sudo systemctl daemon-reload + +### Cleaning up the `system-service` service files + +```bash +sudo rm -f /etc/systemd/system/vf-switcher.service +sudo rm -f /usr/bin/vf-netns-switcher.sh +sudo systemctl daemon-reload ``` -### Known limitations / issues +## Known limitations / issues + * Webhooks are disabled by default when testing diff --git a/doc/testing-virtual-machine.md b/doc/testing-virtual-machine.md index f8b3542431..a2988b351f 100644 --- a/doc/testing-virtual-machine.md +++ b/doc/testing-virtual-machine.md @@ -1,16 +1,19 @@ -## E2E conformance test +# E2E conformance test -It's possible to use QEMU to test the SR-IOV operator on a virtual kubernetes/openshift cluster. -Using the IGB model network driver allow to create virtual functions on the virtual system +It's possible to use QEMU to test the SR-IOV operator on a virtual +kubernetes/openshift cluster. Using the IGB model network driver allow to +create virtual functions on the virtual system ## How to test -First you will need to enable the `DEV_MODE` via the operator environment variable. -Second step is to add the intel virtual nic to the supported nics configmap. +First you will need to enable the `DEV_MODE` via the operator environment +variable. Second step is to add the intel virtual nic to the supported nics +configmap. -Another requirement is to load the vfio kernel module with no_iommu configuration. Example systemd: +Another requirement is to load the vfio kernel module with no_iommu +configuration. Example systemd: -``` +```text [Unit] Description=vfio no-iommu Before=kubelet.service crio.service node-valid-hostname.service @@ -27,35 +30,39 @@ WantedBy=network-online.target ``` ### Prerequisites -* kcli - deployment tool (https://github.com/karmab/kcli) -* virsh + +* kcli - deployment tool () +* virsh * qemu > 8.1 * libvirt > 9 * podman * make * go -*Note:* For openshift you will also need a pull secret that you can download from the [Redhat Console](https://console.redhat.com/) +*Note:* For openshift you will also need a pull secret that you can download +from the [Redhat Console](https://console.redhat.com/) ## Deploy the cluster -use the deployment [script](../hack/run-e2e-conformance-virtual-cluster.sh), this will deploy a k8s cluster -compile the operator images and run the e2e tests. +Use the deployment [script](../hack/run-e2e-conformance-virtual-cluster.sh), +this will deploy a k8s cluster, compile the operator images and run the e2e +tests. -example: -``` -SKIP_DELETE=TRUE make test-e2e-conformance-virtual-k8s-cluster +Example: + +```bash +SKIP_DELETE=TRUE make test-e2e-conformance-virtual-k8s-cluster ``` It's also possible to skip the tests and only deploy the cluster running -``` +```bash SKIP_TEST=TRUE SKIP_DELETE=TRUE make test-e2e-conformance-virtual-k8s-cluster ``` To use the cluster after the deployment you need to export the kubeconfig -``` +```bash export KUBECONFIG=$HOME/.kcli/clusters/virtual/auth/kubeconfig ``` @@ -64,5 +71,5 @@ It's possible to also configure the number of workers using `NUM_OF_WORKERS`. *NOTE:* For OCP then min number is 3 and for k8s is 2. To work on the operator you can change the code and rebuild the operator using -`make redeploy-operator-virtual-cluster`. -You need to tell the cluster type for ocp `CLUSTER_TYPE=openshift` and for k8s `CLUSTER_TYPE=kubernetes` +`make redeploy-operator-virtual-cluster`. You need to tell the cluster type +for ocp `CLUSTER_TYPE=openshift` and for k8s `CLUSTER_TYPE=kubernetes`. diff --git a/doc/vdpa.md b/doc/vdpa.md index e15356b847..40a7ae5cd6 100644 --- a/doc/vdpa.md +++ b/doc/vdpa.md @@ -1,13 +1,15 @@ # VDPA -Virtual data path acceleration (vDPA) in essence is an approach to standardize -the NIC SRIOV data plane using the virtio ring layout and placing a single standard -virtio driver in the guest/pod. It’s decoupled from any vendor implementation while -adding a generic control plane and SW infrastructure to support it. Given that it’s -an abstraction layer on top of SRIOV (Single Root I/O Virtualization) it is also -future proof to support emerging technologies such as scalable IOV. +Virtual data path acceleration (vDPA) in essence is an approach to standardize +the NIC SRIOV data plane using the virtio ring layout and placing a single +standard virtio driver in the guest/pod. It’s decoupled from any vendor +implementation while adding a generic control plane and SW infrastructure to +support it. Given that it’s an abstraction layer on top of SRIOV (Single Root +I/O Virtualization) it is also future proof to support emerging technologies +such as scalable IOV. Main aspects of this solution: + - primary interface in the pod is configured as a virtio/vDPA device - OVS HW offload enabled - NIC configured in switchdev mode @@ -95,7 +97,8 @@ spec: ## Verify vDPA is Working -Run ethtool to verify the correctness of the virtio/vDPA interface in the pod: +Run ethtool to verify the correctness of the virtio/vDPA interface in the +pod: ```bash kubectl exec -it vdpa-pod1 -- ethtool -i eth0 diff --git a/doc/vendor-support.md b/doc/vendor-support.md index daffabc770..af8cc05c0b 100644 --- a/doc/vendor-support.md +++ b/doc/vendor-support.md @@ -1,15 +1,16 @@ # Vendor Support -Below is a list of contact points for hardware vendors that are supported by sriov-network operator. -Before contacting the relevant vendor, please open a github issue with relevant information so we -can first see if indeed the issue is hardware related. +Below is a list of contact points for hardware vendors that are supported by +sriov-network operator. Before contacting the relevant vendor, please open a +github issue with relevant information so we can first see if indeed the issue +is hardware related. -Some vendors are actively involved in the development of sriov-network-operator. They are marked -as `active-in-project` in the table below. +Some vendors are actively involved in the development of sriov-network-operator. +They are marked as `active-in-project` in the table below. | Vendor | Contact | -|--------|---------| +| ------ | ------- | | Nvidia(Mellanox) | active-in-project | | Intel | active-in-project | -| HPE(Marvell Qlogic) | BSN_dev_support@hpe.com | -| Marvell | navadiaev@marvell.com | +| HPE(Marvell Qlogic) | | +| Marvell | | diff --git a/go.mod b/go.mod index eabcfa6a52..1047726e73 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/k8snetworkplumbingwg/sriov-network-operator -go 1.22.4 +go 1.22.12 require ( github.com/Masterminds/sprig/v3 v3.2.2 diff --git a/pkg/apply/merge_test.go b/pkg/apply/merge_test.go index ecf2fd98d2..432523ad72 100644 --- a/pkg/apply/merge_test.go +++ b/pkg/apply/merge_test.go @@ -14,6 +14,7 @@ import ( // Namespaces use the "generic" logic; deployments and services // have custom logic func TestMergeNamespace(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -58,6 +59,7 @@ metadata: } func TestMergeDeployment(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -108,6 +110,7 @@ metadata: } func TestMergeOne(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -140,6 +143,7 @@ metadata: } func TestMergeNilCur(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -176,6 +180,7 @@ metadata: } func TestMergeNilMeta(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -198,6 +203,7 @@ metadata: } func TestMergeNilUpd(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -234,6 +240,7 @@ metadata: } func TestMergeService(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -261,6 +268,7 @@ spec: } func TestMergeServiceAccount(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` @@ -294,6 +302,7 @@ metadata: } func TestMergeWebHookCABundle(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) cur := UnstructuredFromYaml(t, ` diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 53fe82b8ba..b3270d753a 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -3,7 +3,6 @@ package daemon import ( "context" "fmt" - "math/rand" "reflect" "sync" "time" @@ -190,7 +189,6 @@ func (dn *Daemon) Run(stopCh <-chan struct{}, exitCh <-chan error) error { UpdateFunc: dn.operatorConfigChangeHandler, }) - rand.Seed(time.Now().UnixNano()) go cfgInformer.Run(dn.stopCh) time.Sleep(5 * time.Second) go informer.Run(dn.stopCh) @@ -353,7 +351,7 @@ func (dn *Daemon) nodeStateSyncHandler() error { // if the service doesn't exist we should continue to let the k8s plugin to create the service files // this is only for k8s base environments, for openshift the sriov-operator creates a machine config to will apply // the system service and reboot the node the config-daemon doesn't need to do anything. - if !(serviceEnabled && postNetworkServiceEnabled) { + if !serviceEnabled || !postNetworkServiceEnabled { sriovResult = &systemd.SriovResult{SyncStatus: consts.SyncStatusFailed, LastSyncError: fmt.Sprintf("some sriov systemd services are not available on node: "+ "sriov-config available:%t, sriov-config-post-network available:%t", serviceEnabled, postNetworkServiceEnabled)} @@ -714,8 +712,13 @@ func (dn *Daemon) restartDevicePluginPod() error { return err } - if err := wait.PollImmediateUntil(3*time.Second, func() (bool, error) { - _, err := dn.kubeClient.CoreV1().Pods(vars.Namespace).Get(context.Background(), podToDelete, metav1.GetOptions{}) + ctx, cancel := context.WithCancel(context.Background()) + go func() { + <-dn.stopCh + cancel() + }() + if err := wait.PollUntilContextCancel(ctx, 3*time.Second, true, func(ctx context.Context) (bool, error) { + _, err := dn.kubeClient.CoreV1().Pods(vars.Namespace).Get(ctx, podToDelete, metav1.GetOptions{}) if errors.IsNotFound(err) { log.Log.Info("restartDevicePluginPod(): device plugin pod exited") return true, nil @@ -727,7 +730,7 @@ func (dn *Daemon) restartDevicePluginPod() error { log.Log.Info("restartDevicePluginPod(): waiting for device plugin pod to exit", "pod-name", podToDelete) } return false, nil - }, dn.stopCh); err != nil { + }); err != nil { log.Log.Error(err, "restartDevicePluginPod(): failed to wait for checking pod deletion") return err } diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index 42eeb29283..eef12e0bc9 100644 --- a/pkg/daemon/writer.go +++ b/pkg/daemon/writer.go @@ -227,8 +227,10 @@ func (w *NodeStateStatusWriter) recordStatusChangeEvent(oldStatus, newStatus, la func (w *NodeStateStatusWriter) getNodeState() (*sriovnetworkv1.SriovNetworkNodeState, error) { var lastErr error var n *sriovnetworkv1.SriovNetworkNodeState - err := wait.PollImmediate(10*time.Second, 5*time.Minute, func() (bool, error) { - n, lastErr = w.client.SriovnetworkV1().SriovNetworkNodeStates(vars.Namespace).Get(context.Background(), vars.NodeName, metav1.GetOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + err := wait.PollUntilContextCancel(ctx, 10*time.Second, true, func(ctx context.Context) (bool, error) { + n, lastErr = w.client.SriovnetworkV1().SriovNetworkNodeStates(vars.Namespace).Get(ctx, vars.NodeName, metav1.GetOptions{}) if lastErr == nil { return true, nil } @@ -238,7 +240,7 @@ func (w *NodeStateStatusWriter) getNodeState() (*sriovnetworkv1.SriovNetworkNode return false, nil }) if err != nil { - if err == wait.ErrWaitTimeout { + if wait.Interrupted(err) { return nil, errors.Wrapf(lastErr, "Timed out trying to fetch node %s", vars.NodeName) } return nil, err diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index e14cd0329b..9202135aa3 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -57,7 +57,7 @@ func (fg *featureGate) String() string { var result strings.Builder var sep string for k, v := range fg.state { - result.WriteString(fmt.Sprintf("%s%s:%t", sep, k, v)) + fmt.Fprintf(&result, "%s%s:%t", sep, k, v) sep = ", " } return result.String() diff --git a/pkg/host/internal/kernel/kernel.go b/pkg/host/internal/kernel/kernel.go index 95a5c44a54..7d24964480 100644 --- a/pkg/host/internal/kernel/kernel.go +++ b/pkg/host/internal/kernel/kernel.go @@ -60,7 +60,7 @@ func (k *kernel) IsKernelModuleLoaded(kernelModuleName string) (bool, error) { log.Log.V(2).Info("IsKernelModuleLoaded():", "stdout", stdout) if len(stderr) != 0 { log.Log.Error(err, "IsKernelModuleLoaded(): failed to check if kernel module is loaded", "name", kernelModuleName, "stderr", stderr) - return false, fmt.Errorf(stderr) + return false, fmt.Errorf("%s", stderr) } if len(stdout) != 0 { @@ -266,7 +266,7 @@ func (k *kernel) CheckRDMAEnabled() (bool, error) { _, stderr, mlx5Err := k.utilsHelper.RunCommand("/bin/sh", "-c", fmt.Sprintf("%s lsmod | grep --quiet 'mlx5_core'", chrootDefinition)) if mlx5Err != nil && len(stderr) != 0 { log.Log.Error(mlx5Err, "CheckRDMAEnabled(): failed to check for kernel module 'mlx5_core'", "stderr", stderr) - return false, fmt.Errorf(stderr) + return false, fmt.Errorf("%s", stderr) } if mlx5Err != nil { @@ -284,7 +284,7 @@ func (k *kernel) rdmaModulesAreLoaded() (bool, error) { _, stderr, err := k.utilsHelper.RunCommand("/bin/sh", "-c", fmt.Sprintf("%s lsmod | grep --quiet '\\(^ib\\|^rdma\\)'", chrootDefinition)) if err != nil && len(stderr) != 0 { log.Log.Error(err, "rdmaModulesAreLoaded(): fail to check if ib and rdma kernel modules are loaded", "stderr", stderr) - return false, fmt.Errorf(stderr) + return false, fmt.Errorf("%s", stderr) } if err != nil { diff --git a/pkg/host/internal/sriov/sriov.go b/pkg/host/internal/sriov/sriov.go index 3e5989bae3..855d12b009 100644 --- a/pkg/host/internal/sriov/sriov.go +++ b/pkg/host/internal/sriov/sriov.go @@ -1,6 +1,7 @@ package sriov import ( + "context" "errors" "fmt" "os" @@ -93,7 +94,8 @@ func (s *sriov) SetSriovNumVfs(pciAddr string, numVfs int) error { func (s *sriov) ResetSriovDevice(ifaceStatus sriovnetworkv1.InterfaceExt) error { log.Log.V(2).Info("ResetSriovDevice(): reset SRIOV device", "address", ifaceStatus.PciAddress) - if ifaceStatus.LinkType == consts.LinkTypeETH { + switch ifaceStatus.LinkType { + case consts.LinkTypeETH: var mtu int eswitchMode := sriovnetworkv1.ESwithModeLegacy is := sriovnetworkv1.InitialState.GetInterfaceStateByPciAddress(ifaceStatus.PciAddress) @@ -111,7 +113,7 @@ func (s *sriov) ResetSriovDevice(ifaceStatus sriovnetworkv1.InterfaceExt) error if err := s.setEswitchModeAndNumVFs(ifaceStatus.PciAddress, eswitchMode, 0); err != nil { return err } - } else if ifaceStatus.LinkType == consts.LinkTypeIB { + case consts.LinkTypeIB: if err := s.SetSriovNumVfs(ifaceStatus.PciAddress, 0); err != nil { return err } @@ -173,7 +175,9 @@ func (s *sriov) VFIsReady(pciAddr string) (netlink.Link, error) { log.Log.Info("VFIsReady()", "device", pciAddr) var err error var vfLink netlink.Link - err = wait.PollImmediate(time.Second, 10*time.Second, func() (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + err = wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { vfIndex, err := s.networkHelper.GetInterfaceIndex(pciAddr) if err != nil { log.Log.Error(err, "VFIsReady(): invalid index number") @@ -408,7 +412,7 @@ func (s *sriov) checkExternallyManagedPF(iface *sriovnetworkv1.Interface) error "functions %d but the policy is configured as ExternallyManaged for device %s", iface.NumVfs, currentNumVfs, iface.PciAddress) log.Log.Error(nil, errMsg) - return fmt.Errorf(errMsg) + return fmt.Errorf("%s", errMsg) } currentEswitchMode := s.GetNicSriovMode(iface.PciAddress) expectedEswitchMode := sriovnetworkv1.GetEswitchModeFromSpec(iface) @@ -416,7 +420,7 @@ func (s *sriov) checkExternallyManagedPF(iface *sriovnetworkv1.Interface) error errMsg := fmt.Sprintf("checkExternallyManagedPF(): requested ESwitchMode mode \"%s\" is not equal to configured \"%s\" "+ "but the policy is configured as ExternallyManaged for device %s", expectedEswitchMode, currentEswitchMode, iface.PciAddress) log.Log.Error(nil, errMsg) - return fmt.Errorf(errMsg) + return fmt.Errorf("%s", errMsg) } currentMtu := s.networkHelper.GetNetdevMTU(iface.PciAddress) if iface.Mtu > 0 && iface.Mtu > currentMtu { @@ -924,12 +928,14 @@ func (s *sriov) GetLinkType(name string) string { } func (s *sriov) encapTypeToLinkType(encapType string) string { - if encapType == "ether" { + switch encapType { + case "ether": return consts.LinkTypeETH - } else if encapType == "infiniband" { + case "infiniband": return consts.LinkTypeIB + default: + return "" } - return "" } // create required udev rules for PF: diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index 948459a7f0..2223324e30 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -458,16 +458,17 @@ func (p *GenericPlugin) addVfioDesiredKernelArg(state *sriovnetworkv1.SriovNetwo } func (p *GenericPlugin) configRdmaKernelArg(state *sriovnetworkv1.SriovNetworkNodeState) error { - if state.Spec.System.RdmaMode == "" { + switch state.Spec.System.RdmaMode { + case "": p.disableDesiredKernelArgs(consts.KernelArgRdmaExclusive) p.disableDesiredKernelArgs(consts.KernelArgRdmaShared) - } else if state.Spec.System.RdmaMode == "shared" { + case "shared": p.enableDesiredKernelArgs(consts.KernelArgRdmaShared) p.disableDesiredKernelArgs(consts.KernelArgRdmaExclusive) - } else if state.Spec.System.RdmaMode == "exclusive" { + case "exclusive": p.enableDesiredKernelArgs(consts.KernelArgRdmaExclusive) p.disableDesiredKernelArgs(consts.KernelArgRdmaShared) - } else { + default: err := fmt.Errorf("unexpected rdma mode: %s", state.Spec.System.RdmaMode) log.Log.Error(err, "generic-plugin configRdmaKernelArg(): failed to configure kernel arguments for rdma") return err diff --git a/pkg/render/README.md b/pkg/render/README.md index c7c4e1c103..6ef03d0bf1 100644 --- a/pkg/render/README.md +++ b/pkg/render/README.md @@ -1,4 +1,8 @@ # Template rendering -The operator will render all files in a directory that end with ".json" or ".yaml". The files will be passed through the [Go templating engine](https://golang.org/pkg/text/template/). -The aim is to mimic the parsing behavior of `kubectl create -f ` as much as reasonably possible. +The operator will render all files in a directory that end with ".json" or +".yaml". The files will be passed through the +[Go templating engine](https://golang.org/pkg/text/template/). + +The aim is to mimic the parsing behavior of `kubectl create -f ` as much +as reasonably possible. diff --git a/pkg/render/render.go b/pkg/render/render.go index aa26b018ba..0e98d1f6ed 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -60,7 +60,7 @@ func RenderDir(manifestDir string, d *RenderData) ([]*unstructured.Unstructured, } // Skip non-manifest files - if !(strings.HasSuffix(path, ".yml") || strings.HasSuffix(path, ".yaml") || strings.HasSuffix(path, ".json")) { + if !strings.HasSuffix(path, ".yml") && !strings.HasSuffix(path, ".yaml") && !strings.HasSuffix(path, ".json") { return nil } diff --git a/pkg/render/render_test.go b/pkg/render/render_test.go index be4d0f0bdd..10b7107f65 100644 --- a/pkg/render/render_test.go +++ b/pkg/render/render_test.go @@ -8,6 +8,7 @@ import ( // TestRenderSimple tests rendering a single object with no templates func TestRenderSimple(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) d := MakeRenderData() @@ -42,6 +43,7 @@ func TestRenderSimple(t *testing.T) { } func TestRenderMultiple(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) p := "testdata/manifests/multiple.yaml" @@ -58,6 +60,7 @@ func TestRenderMultiple(t *testing.T) { } func TestTemplate(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) p := "testdata/manifests/template.yaml" @@ -89,6 +92,7 @@ func TestTemplate(t *testing.T) { // object can also occur in the particular case shown in the testfile below when minus is missing at the end of the // first expression (i.e. {{- if .Enable }}). func TestTemplateWithEmptyObject(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) p := "testdata/manifests/template_with_empty_object.yaml" @@ -106,6 +110,7 @@ func TestTemplateWithEmptyObject(t *testing.T) { } func TestRenderDir(t *testing.T) { + t.Parallel() g := NewGomegaWithT(t) d := MakeRenderData() diff --git a/pkg/utils/cluster.go b/pkg/utils/cluster.go index 5f9aa7065c..5eed114751 100644 --- a/pkg/utils/cluster.go +++ b/pkg/utils/cluster.go @@ -27,9 +27,10 @@ const ( func getNodeRole(node corev1.Node) string { for k := range node.Labels { - if k == workerNodeLabelKey { + switch k { + case workerNodeLabelKey: return workerRoleName - } else if k == masterNodeLabelKey || k == controlPlaneNodeLabelKey { + case masterNodeLabelKey, controlPlaneNodeLabelKey: return masterRoleName } } diff --git a/pkg/utils/shutdown.go b/pkg/utils/shutdown.go index f8f9618d4b..a18d119275 100644 --- a/pkg/utils/shutdown.go +++ b/pkg/utils/shutdown.go @@ -35,7 +35,7 @@ func updateFinalizers() { shutdownLog.Error(err, "Failed to list SriovNetworks") } else { for _, instance := range networkList.Items { - if instance.ObjectMeta.Finalizers == nil || len(instance.ObjectMeta.Finalizers) == 0 { + if len(instance.Finalizers) == 0 { continue } if err != nil { @@ -43,7 +43,7 @@ func updateFinalizers() { } shutdownLog.Info("Clearing finalizers on SriovNetwork ", "namespace", instance.GetNamespace(), "name", instance.GetName()) var found bool - instance.ObjectMeta.Finalizers, found = sriovnetworkv1.RemoveString(sriovnetworkv1.NETATTDEFFINALIZERNAME, instance.ObjectMeta.Finalizers) + instance.Finalizers, found = sriovnetworkv1.RemoveString(sriovnetworkv1.NETATTDEFFINALIZERNAME, instance.Finalizers) if found { _, err = sriovNetworkClient.SriovNetworks(instance.GetNamespace()).Update(context.TODO(), &instance, metav1.UpdateOptions{}) if err != nil { diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 739d2fc613..a3be8fe83d 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -196,7 +196,7 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol if rngEnd < rngSt { return false, fmt.Errorf("failed to parse %s PF name nicSelector, end range shall not be smaller than start range", pf) } - if !(rngEnd < cr.Spec.NumVfs) { + if rngEnd >= cr.Spec.NumVfs { return false, fmt.Errorf("failed to parse %s PF name nicSelector, end range exceeds the maximum VF index ", pf) } } @@ -333,7 +333,7 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s return nil, fmt.Errorf("MTU(%d) in CR %s is higher than the MTU for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu) } - if policy.Spec.LinkType != "" && strings.ToLower(policy.Spec.LinkType) != strings.ToLower(iface.LinkType) { + if policy.Spec.LinkType != "" && !strings.EqualFold(policy.Spec.LinkType, iface.LinkType) { return nil, fmt.Errorf("LinkType(%s) in CR %s is not equal to the LinkType for the PF externally value(%s)", policy.Spec.LinkType, policy.GetName(), iface.LinkType) } } diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go index 73c53412c2..65c1f58282 100644 --- a/test/conformance/tests/test_sriov_operator.go +++ b/test/conformance/tests/test_sriov_operator.go @@ -24,7 +24,7 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" @@ -453,7 +453,7 @@ var _ = Describe("[sriov] operator", func() { }, (10+snoTimeoutMultiplier*110)*time.Second, 1*time.Second).ShouldNot(HaveOccurred()) podDefinition := pod.DefineWithNetworks([]string{sriovNetwork.Name}) - podDefinition.ObjectMeta.Labels = map[string]string{"anyname": "anyvalue"} + podDefinition.Labels = map[string]string{"anyname": "anyvalue"} created, err := clients.Pods(namespaces.Test).Create(context.Background(), podDefinition, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred()) @@ -522,7 +522,7 @@ var _ = Describe("[sriov] operator", func() { } err = clients.Pods(namespaces.Test).Delete(context.Background(), podObj.Name, metav1.DeleteOptions{ - GracePeriodSeconds: pointer.Int64Ptr(0)}) + GracePeriodSeconds: ptr.To[int64](0)}) Expect(err).ToNot(HaveOccurred()) return found @@ -1006,7 +1006,7 @@ var _ = Describe("[sriov] operator", func() { }, 2*time.Minute, 10*time.Second).Should(BeTrue(), "Error to detect Required Event") By("Delete first pod and release all VFs") err = clients.Pods(namespaces.Test).Delete(context.Background(), runningPodA.Name, metav1.DeleteOptions{ - GracePeriodSeconds: pointer.Int64Ptr(0), + GracePeriodSeconds: ptr.To[int64](0), }) Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("Error to delete pod %s", runningPodA.Name)) By("Checking that second pod is able to use released VF") diff --git a/test/util/cluster/cluster.go b/test/util/cluster/cluster.go index 3669a4b6bc..c79b56c852 100644 --- a/test/util/cluster/cluster.go +++ b/test/util/cluster/cluster.go @@ -14,7 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" @@ -383,15 +383,17 @@ func GetNodeSecureBootState(clients *testclient.ClientSet, nodeName, namespace s } defer func() { - err = clients.Pods(namespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{GracePeriodSeconds: pointer.Int64Ptr(0)}) + err = clients.Pods(namespace).Delete(context.Background(), created.Name, metav1.DeleteOptions{GracePeriodSeconds: ptr.To[int64](0)}) if err != nil { err = fmt.Errorf("failed to remove the check secure boot status pod for node %s: %v", nodeName, err) } }() var runningPod *corev1.Pod - err = wait.PollImmediate(time.Second, 3*time.Minute, func() (bool, error) { - runningPod, err = clients.Pods(namespace).Get(context.Background(), created.Name, metav1.GetOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + err = wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { + runningPod, err = clients.Pods(namespace).Get(ctx, created.Name, metav1.GetOptions{}) if err != nil { return false, err } diff --git a/test/util/discovery/discovery.go b/test/util/discovery/discovery.go index 713cb15cb8..022f878290 100644 --- a/test/util/discovery/discovery.go +++ b/test/util/discovery/discovery.go @@ -38,11 +38,11 @@ func DiscoveredResources(clients *client.ClientSet, sriovInfos *cluster.EnabledN } resourceName := policy.Spec.ResourceName for _, node := range nodes { - sriovDeviceList, err := sriovInfos.FindSriovDevices(node.ObjectMeta.Name) + sriovDeviceList, err := sriovInfos.FindSriovDevices(node.Name) if err != nil { continue } - device, ok := filterDevices(node.ObjectMeta.Name, sriovDeviceList) + device, ok := filterDevices(node.Name, sriovDeviceList) if !ok { continue } @@ -52,7 +52,7 @@ func DiscoveredResources(clients *client.ClientSet, sriovInfos *cluster.EnabledN if resourceCount > preferredResourceCount { preferredResourceCount = resourceCount preferredResourceName = resourceName - preferredNode = node.ObjectMeta.Name + preferredNode = node.Name preferredDevice = device } } diff --git a/test/util/namespaces/namespaces.go b/test/util/namespaces/namespaces.go index 5ed106398e..bc9b15b1d8 100644 --- a/test/util/namespaces/namespaces.go +++ b/test/util/namespaces/namespaces.go @@ -11,7 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" @@ -30,8 +30,10 @@ var inhibitSecurityAdmissionLabels = map[string]string{ // WaitForDeletion waits until the namespace will be removed from the cluster func WaitForDeletion(cs *testclient.ClientSet, nsName string, timeout time.Duration) error { - return wait.PollImmediate(time.Second, timeout, func() (bool, error) { - _, err := cs.Namespaces().Get(context.Background(), nsName, metav1.GetOptions{}) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + return wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { + _, err := cs.Namespaces().Get(ctx, nsName, metav1.GetOptions{}) if k8serrors.IsNotFound(err) { return true, nil } @@ -80,7 +82,7 @@ func CleanPods(namespace string, cs *testclient.ClientSet) error { return nil } err := cs.Pods(namespace).DeleteCollection(context.Background(), metav1.DeleteOptions{ - GracePeriodSeconds: pointer.Int64Ptr(0), + GracePeriodSeconds: ptr.To[int64](0), }, metav1.ListOptions{}) if err != nil { return fmt.Errorf("failed to delete pods %v", err) @@ -130,9 +132,11 @@ func CleanNetworks(operatorNamespace string, cs *testclient.ClientSet) error { } func waitForSriovNetworkDeletion(operatorNamespace string, cs *testclient.ClientSet, timeout time.Duration) error { - return wait.PollImmediate(time.Second, timeout, func() (bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + return wait.PollUntilContextCancel(ctx, time.Second, true, func(ctx context.Context) (bool, error) { networks := sriovv1.SriovNetworkList{} - err := cs.List(context.Background(), + err := cs.List(ctx, &networks, runtimeclient.InNamespace(operatorNamespace)) if err != nil { diff --git a/test/util/network/network.go b/test/util/network/network.go index fd0de835b1..2450a5236e 100644 --- a/test/util/network/network.go +++ b/test/util/network/network.go @@ -93,7 +93,7 @@ func CreateSriovPolicy(clientSet *testclient.ClientSet, generatedName string, op func GetNicsByPrefix(pod *k8sv1.Pod, ifcPrefix string) ([]string, error) { var nets []Network nics := []string{} - err := json.Unmarshal([]byte(pod.ObjectMeta.Annotations[netattdefv1.NetworkStatusAnnot]), &nets) + err := json.Unmarshal([]byte(pod.Annotations[netattdefv1.NetworkStatusAnnot]), &nets) if err != nil { return nil, err } @@ -108,7 +108,7 @@ func GetNicsByPrefix(pod *k8sv1.Pod, ifcPrefix string) ([]string, error) { // GetSriovNicIPs returns the list of ip addresses related to the given // interface name for the given pod. func GetSriovNicIPs(pod *k8sv1.Pod, ifcName string) ([]string, error) { - networksStatus, ok := pod.ObjectMeta.Annotations[netattdefv1.NetworkStatusAnnot] + networksStatus, ok := pod.Annotations[netattdefv1.NetworkStatusAnnot] if !ok { return nil, fmt.Errorf("pod [%s] has no annotation `%s`", netattdefv1.NetworkStatusAnnot, pod.Name) } diff --git a/test/util/network/network_test.go b/test/util/network/network_test.go index f28b497f37..181deba5ba 100644 --- a/test/util/network/network_test.go +++ b/test/util/network/network_test.go @@ -11,6 +11,7 @@ import ( ) func TestGetSriovNicIPs(t *testing.T) { + t.Parallel() networkStatus := `[{ "name": "network1", "interface": "eth0", @@ -59,6 +60,7 @@ func TestGetSriovNicIPs(t *testing.T) { } func TestGetSriovNicIPsErrors(t *testing.T) { + t.Parallel() p := &k8sv1.Pod{} _, err := GetSriovNicIPs(p, "eth0") assert.Error(t, err) diff --git a/test/util/pod/pod.go b/test/util/pod/pod.go index c99ae00b2d..e691b0b49e 100644 --- a/test/util/pod/pod.go +++ b/test/util/pod/pod.go @@ -11,7 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/remotecommand" - "k8s.io/utils/pointer" + "k8s.io/utils/ptr" testclient "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/client" "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util/images" @@ -26,7 +26,7 @@ func GetDefinition() *corev1.Pod { GenerateName: "testpod-", Namespace: namespaces.Test}, Spec: corev1.PodSpec{ - TerminationGracePeriodSeconds: pointer.Int64Ptr(0), + TerminationGracePeriodSeconds: ptr.To[int64](0), Containers: []corev1.Container{{Name: "test", Image: images.Test(), SecurityContext: &corev1.SecurityContext{ diff --git a/test/util/util.go b/test/util/util.go index 5103af78fc..18a45c606c 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -36,10 +36,12 @@ var ( func WaitForSriovNetworkNodeStateReady(nodeState *sriovnetworkv1.SriovNetworkNodeState, client client.Client, namespace, name string, retryInterval, timeout time.Duration) error { time.Sleep(30 * time.Second) - err := wait.PollImmediate(retryInterval, timeout, func() (done bool, err error) { - ctx, cancel := goctx.WithTimeout(goctx.Background(), APITimeout) - defer cancel() - err = client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, nodeState) + ctx, cancel := goctx.WithTimeout(goctx.Background(), timeout) + defer cancel() + err := wait.PollUntilContextCancel(ctx, retryInterval, true, func(ctx goctx.Context) (done bool, err error) { + reqCtx, reqCancel := goctx.WithTimeout(ctx, APITimeout) + defer reqCancel() + err = client.Get(reqCtx, types.NamespacedName{Name: name, Namespace: namespace}, nodeState) if err != nil { if errors.IsNotFound(err) { return false, nil @@ -60,10 +62,12 @@ func WaitForSriovNetworkNodeStateReady(nodeState *sriovnetworkv1.SriovNetworkNod } func WaitForDaemonSetReady(ds *appsv1.DaemonSet, client client.Client, namespace, name string, retryInterval, timeout time.Duration) error { - err := wait.PollImmediate(retryInterval, timeout, func() (done bool, err error) { - ctx, cancel := goctx.WithTimeout(goctx.Background(), APITimeout) - defer cancel() - err = client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, ds) + ctx, cancel := goctx.WithTimeout(goctx.Background(), timeout) + defer cancel() + err := wait.PollUntilContextCancel(ctx, retryInterval, true, func(ctx goctx.Context) (done bool, err error) { + reqCtx, reqCancel := goctx.WithTimeout(ctx, APITimeout) + defer reqCancel() + err = client.Get(reqCtx, types.NamespacedName{Name: name, Namespace: namespace}, ds) if err != nil { if errors.IsNotFound(err) { return false, nil @@ -85,10 +89,12 @@ func WaitForDaemonSetReady(ds *appsv1.DaemonSet, client client.Client, namespace } func WaitForNamespacedObject(obj client.Object, client client.Client, namespace, name string, retryInterval, timeout time.Duration) error { - err := wait.PollImmediate(retryInterval, timeout, func() (done bool, err error) { - ctx, cancel := goctx.WithTimeout(goctx.Background(), APITimeout) - defer cancel() - err = client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, obj) + ctx, cancel := goctx.WithTimeout(goctx.Background(), timeout) + defer cancel() + err := wait.PollUntilContextCancel(ctx, retryInterval, true, func(ctx goctx.Context) (done bool, err error) { + reqCtx, reqCancel := goctx.WithTimeout(ctx, APITimeout) + defer reqCancel() + err = client.Get(reqCtx, types.NamespacedName{Name: name, Namespace: namespace}, obj) if err != nil { if errors.IsNotFound(err) { return false, nil @@ -106,10 +112,12 @@ func WaitForNamespacedObject(obj client.Object, client client.Client, namespace, } func WaitForNamespacedObjectDeleted(obj client.Object, client client.Client, namespace, name string, retryInterval, timeout time.Duration) error { - err := wait.PollImmediate(retryInterval, timeout, func() (done bool, err error) { - ctx, cancel := goctx.WithTimeout(goctx.Background(), APITimeout) - defer cancel() - err = client.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, obj) + ctx, cancel := goctx.WithTimeout(goctx.Background(), timeout) + defer cancel() + err := wait.PollUntilContextCancel(ctx, retryInterval, true, func(ctx goctx.Context) (done bool, err error) { + reqCtx, reqCancel := goctx.WithTimeout(ctx, APITimeout) + defer reqCancel() + err = client.Get(reqCtx, types.NamespacedName{Name: name, Namespace: namespace}, obj) if err != nil { if errors.IsNotFound(err) { return true, nil @@ -209,11 +217,11 @@ func TriggerSriovOperatorConfigReconcile(client client.Client, operatorNamespace return err } - if config.ObjectMeta.Labels == nil { - config.ObjectMeta.Labels = make(map[string]string) + if config.Labels == nil { + config.Labels = make(map[string]string) } - config.ObjectMeta.Labels["trigger-test"] = uuid.NewString() + config.Labels["trigger-test"] = uuid.NewString() return client.Update(goctx.Background(), config) } diff --git a/test/validation/tests/test_validation.go b/test/validation/tests/test_validation.go index 3c2e0379bd..1a7d647744 100644 --- a/test/validation/tests/test_validation.go +++ b/test/validation/tests/test_validation.go @@ -69,22 +69,22 @@ var _ = Describe("validation", func() { It("Should have the sriov CRDs available in the cluster", func() { crd := &apiext.CustomResourceDefinition{} - err := clients.Client.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworkNodePolicies}, crd) + err := clients.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworkNodePolicies}, crd) Expect(err).ToNot(HaveOccurred()) - err = clients.Client.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworkNodeStates}, crd) + err = clients.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworkNodeStates}, crd) Expect(err).ToNot(HaveOccurred()) - err = clients.Client.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworks}, crd) + err = clients.Get(context.TODO(), goclient.ObjectKey{Name: sriovNetworks}, crd) Expect(err).ToNot(HaveOccurred()) - err = clients.Client.Get(context.TODO(), goclient.ObjectKey{Name: sriovOperatorConfigs}, crd) + err = clients.Get(context.TODO(), goclient.ObjectKey{Name: sriovOperatorConfigs}, crd) Expect(err).ToNot(HaveOccurred()) }) It("should deploy the injector pod if requested", func() { operatorConfig := &sriovv1.SriovOperatorConfig{} - err := clients.Client.Get(context.TODO(), goclient.ObjectKey{Name: "default", Namespace: operatorNamespace}, operatorConfig) + err := clients.Get(context.TODO(), goclient.ObjectKey{Name: "default", Namespace: operatorNamespace}, operatorConfig) Expect(err).ToNot(HaveOccurred()) if operatorConfig.Spec.EnableInjector {