From e804b965e647de41be57972b11637eaf5fa9bb67 Mon Sep 17 00:00:00 2001 From: Mohamed Mahmoud Date: Wed, 23 Feb 2022 19:09:18 -0500 Subject: [PATCH 01/25] Move memory-trimming-on-compaction out of dbchecker to nbdb and sbdb init In order to avoid any potential bringup race conditions between dbchecker container and sbdb/nbdb we move the memory trimming check and enable to start of each db. Signed-off-by: Mohamed Mahmoud --- dist/images/ovnkube.sh | 28 +++++++++++++++++-- .../cmd/ovndbchecker/ovndbchecker.go | 4 --- .../pkg/ovndbmanager/ovndbmanager.go | 23 --------------- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/dist/images/ovnkube.sh b/dist/images/ovnkube.sh index b4855d68bf..f283590237 100755 --- a/dist/images/ovnkube.sh +++ b/dist/images/ovnkube.sh @@ -681,6 +681,19 @@ EOF fi } +function memory_trim_on_compaction_supported { + if [[ $1 == "nbdb" ]]; then + mem_trim_check=$(ovn-appctl -t ${OVN_RUNDIR}/ovnnb_db.ctl list-commands | grep "memory-trim-on-compaction") + elif [[ $1 == "sbdb" ]]; then + mem_trim_check=$(ovn-appctl -t ${OVN_RUNDIR}/ovnsb_db.ctl list-commands | grep "memory-trim-on-compaction") + fi + if [[ ${mem_trim_check} != "" ]]; then + return $(/bin/true) + else + return $(/bin/false) + fi +} + # v3 - run nb_ovsdb in a separate container nb-ovsdb() { trap 'ovsdb_cleanup nb' TERM @@ -707,10 +720,14 @@ nb-ovsdb() { echo "=============== nb-ovsdb ========== reconfigured for SSL" } ovn-nbctl --inactivity-probe=0 set-connection p${transport}:${ovn_nb_port}:$(bracketify ${ovn_db_host}) - + if memory_trim_on_compaction_supported "nbdb" + then + # Enable NBDB memory trimming on DB compaction, Every 10mins DBs are compacted + # memory on the heap is freed, when enable memory trimmming freed memory will go back to OS. + ovn-appctl -t ${OVN_RUNDIR}/ovnnb_db.ctl ovsdb-server/memory-trim-on-compaction on + fi tail --follow=name ${OVN_LOGDIR}/ovsdb-server-nb.log & ovn_tail_pid=$! - process_healthy ovnnb_db ${ovn_tail_pid} echo "=============== run nb_ovsdb ========== terminated" } @@ -743,7 +760,12 @@ sb-ovsdb() { # create the ovnkube-db endpoints wait_for_event attempts=10 check_ovnkube_db_ep ${ovn_db_host} ${ovn_nb_port} set_ovnkube_db_ep ${ovn_db_host} - + if memory_trim_on_compaction_supported "sbdb" + then + # Enable SBDB memory trimming on DB compaction, Every 10mins DBs are compacted + # memory on the heap is freed, when enable memory trimmming freed memory will go back to OS. + ovn-appctl -t ${OVN_RUNDIR}/ovnsb_db.ctl ovsdb-server/memory-trim-on-compaction on + fi tail --follow=name ${OVN_LOGDIR}/ovsdb-server-sb.log & ovn_tail_pid=$! diff --git a/go-controller/cmd/ovndbchecker/ovndbchecker.go b/go-controller/cmd/ovndbchecker/ovndbchecker.go index a5a7b9c9cf..456c6b9e16 100644 --- a/go-controller/cmd/ovndbchecker/ovndbchecker.go +++ b/go-controller/cmd/ovndbchecker/ovndbchecker.go @@ -182,10 +182,6 @@ func runOvnKubeDBChecker(ctx *cli.Context) error { return err } - if err = ovndbmanager.EnableDBMemTrimming(); err != nil { - return err - } - stopChan := make(chan struct{}) go ovndbmanager.RunDBChecker( &kube.Kube{ diff --git a/go-controller/pkg/ovndbmanager/ovndbmanager.go b/go-controller/pkg/ovndbmanager/ovndbmanager.go index 9c28407e31..be020b7109 100644 --- a/go-controller/pkg/ovndbmanager/ovndbmanager.go +++ b/go-controller/pkg/ovndbmanager/ovndbmanager.go @@ -340,29 +340,6 @@ func resetRaftDB(db *dbProperties) error { return nil } -// EnableDBMemTrimming enables memory trimming on DB compaction for NBDB and SBDB. Every 10 minutes the DBs are compacted -// and excess memory on the heap is freed. By enabling memory trimming, the freed memory will be returned back to the OS -func EnableDBMemTrimming() error { - out, stderr, err := util.RunOVNNBAppCtlWithTimeout(5, "list-commands") - if err != nil { - return fmt.Errorf("unable to list supported commands for ovn-appctl, stderr: %s, error: %v", stderr, err) - } - if !strings.Contains(out, "memory-trim-on-compaction") { - klog.Warning("memory-trim-on-compaction unsupported in this version of OVN. OVN DBs may experience high " + - "memory growth") - return nil - } - _, stderr, err = util.RunOVNNBAppCtlWithTimeout(5, "ovsdb-server/memory-trim-on-compaction", "on") - if err != nil { - return fmt.Errorf("unable to turn on memory trimming for NB DB, stderr: %s, error: %v", stderr, err) - } - _, stderr, err = util.RunOVNSBAppCtlWithTimeout(5, "ovsdb-server/memory-trim-on-compaction", "on") - if err != nil { - return fmt.Errorf("unable to turn on memory trimming for SB DB, stderr: %s, error: %v", stderr, err) - } - return nil -} - func propertiesForDB(db string) *dbProperties { if strings.Contains(db, "ovnnb") { return &dbProperties{ From 51b624de570acc68461f337f446a90d6eea7fb46 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 25 Feb 2022 11:37:19 -0600 Subject: [PATCH 02/25] gateway: fix test teardown order to really prevent nodeIP manager flake We need to stop the node IP manager before the factory, becuase the node IP manager uses the factory. Fixes: eb73081 node: wait for nodeIP and OpenFlow manager to stop before next test Signed-off-by: Dan Williams --- go-controller/pkg/node/gateway_init_linux_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go-controller/pkg/node/gateway_init_linux_test.go b/go-controller/pkg/node/gateway_init_linux_test.go index c7c7ebf4a2..96d5e08702 100644 --- a/go-controller/pkg/node/gateway_init_linux_test.go +++ b/go-controller/pkg/node/gateway_init_linux_test.go @@ -168,8 +168,8 @@ func shareGatewayInterfaceTest(app *cli.App, testNS ns.NetNS, wg := &sync.WaitGroup{} defer func() { close(stop) - wf.Shutdown() wg.Wait() + wf.Shutdown() }() err = wf.Start() Expect(err).NotTo(HaveOccurred()) @@ -438,8 +438,8 @@ func shareGatewayInterfaceDPUTest(app *cli.App, testNS ns.NetNS, wg := &sync.WaitGroup{} defer func() { close(stop) - wf.Shutdown() wg.Wait() + wf.Shutdown() }() err = wf.Start() Expect(err).NotTo(HaveOccurred()) @@ -748,8 +748,8 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`, wg := &sync.WaitGroup{} defer func() { close(stop) - wf.Shutdown() wg.Wait() + wf.Shutdown() }() err = wf.Start() Expect(err).NotTo(HaveOccurred()) From 78881d5ea6c42fe3d4cb3b5c773de0bc8f39be2c Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Fri, 25 Feb 2022 22:25:22 +0100 Subject: [PATCH 03/25] Add metric to record gateway modes A metric is the first step towards telemetry and we need to know the count of lgw users. This will be a first step towards that. Signed-off-by: Surya Seetharaman --- go-controller/pkg/metrics/master.go | 26 ++++++++++++++++++++++++++ go-controller/pkg/ovn/gateway_init.go | 4 ++++ 2 files changed, 30 insertions(+) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index 6e268e7d1d..f5eb48f280 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -185,6 +185,13 @@ var metricIPsecEnabled = prometheus.NewGauge(prometheus.GaugeOpts{ Help: "Specifies whether IPSec is enabled for this cluster(1) or not enabled for this cluster(0)", }) +var metricEgressRoutingViaHost = prometheus.NewGauge(prometheus.GaugeOpts{ + Namespace: MetricOvnkubeNamespace, + Subsystem: MetricOvnkubeSubsystemMaster, + Name: "egress_routing_via_host", + Help: "Specifies whether egress gateway mode is via host networking stack(1) or not(0)", +}) + var metricEgressFirewallCount = prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: MetricOvnkubeNamespace, Subsystem: MetricOvnkubeSubsystemMaster, @@ -303,6 +310,7 @@ func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclie prometheus.MustRegister(metricEgressFirewallRuleCount) prometheus.MustRegister(metricIPsecEnabled) prometheus.MustRegister(metricEgressFirewallCount) + prometheus.MustRegister(metricEgressRoutingViaHost) registerControlPlaneRecorderMetrics() registerWorkqueueMetrics(MetricOvnkubeNamespace, MetricOvnkubeSubsystemMaster) @@ -410,6 +418,24 @@ func UpdateEgressFirewallRuleCount(count float64) { metricEgressFirewallRuleCount.Add(count) } +// RecordEgressRoutingViaHost records the egress gateway mode of the cluster +// The values are: +// 0: If it is shared gateway mode +// 1: If it is local gateway mode +// 2: invalid mode +func RecordEgressRoutingViaHost() { + if config.Gateway.Mode == config.GatewayModeLocal { + // routingViaHost is enabled + metricEgressRoutingViaHost.Set(1) + } else if config.Gateway.Mode == config.GatewayModeShared { + // routingViaOVN is enabled + metricEgressRoutingViaHost.Set(0) + } else { + // invalid mode + metricEgressRoutingViaHost.Set(2) + } +} + func updateE2ETimestampMetric(ovnNBClient libovsdbclient.Client) { currentTime := time.Now().Unix() // assumption that only first row is relevant in NB_Global table diff --git a/go-controller/pkg/ovn/gateway_init.go b/go-controller/pkg/ovn/gateway_init.go index ecb67735ec..72a2794d65 100644 --- a/go-controller/pkg/ovn/gateway_init.go +++ b/go-controller/pkg/ovn/gateway_init.go @@ -9,6 +9,7 @@ import ( "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/libovsdbops" + "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/metrics" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/nbdb" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" @@ -488,6 +489,9 @@ func (oc *Controller) gatewayInit(nodeName string, clusterIPSubnet []*net.IPNet, } } + // recording gateway mode metrics here after gateway setup is done + metrics.RecordEgressRoutingViaHost() + return nil } From 7d6860a5b41e43ecc7bc916890c8fdd49d1f1288 Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Sat, 26 Feb 2022 15:09:53 +0000 Subject: [PATCH 04/25] Metrics: Remove stale label metric entries We are using labelled (vector) metrics to export info and if the label values change, we still export the old labels values which confuses users because they have no way of determining which labelled metric is the latest. We must reset each vector metric prior to using it to ensure we are only exporting the current state and not old states which should have been scraped previously due to our usage of labels that can be dynamically updated during runtime. This is occurring in the OVN metrics exported by ovnkube-node. Follow on work is needed to fix OVS metrics that are used by ovs-exporter. Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/ovn.go | 5 +++++ go-controller/pkg/metrics/ovn_db.go | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/go-controller/pkg/metrics/ovn.go b/go-controller/pkg/metrics/ovn.go index cfce8e7b58..d1cc2b1ee6 100644 --- a/go-controller/pkg/metrics/ovn.go +++ b/go-controller/pkg/metrics/ovn.go @@ -282,16 +282,21 @@ func setOvnControllerConfigurationMetrics() (err error) { } metricMonitorAll.Set(ovnMonitorValue) case "ovn-encap-ip": + // To update not only values but also labels for metrics, we use Reset() to delete previous labels+value + metricEncapIP.Reset() metricEncapIP.WithLabelValues(fieldValue).Set(1) case "ovn-remote": + metricSbConnectionMethod.Reset() metricSbConnectionMethod.WithLabelValues(fieldValue).Set(1) case "ovn-encap-type": + metricEncapType.Reset() metricEncapType.WithLabelValues(fieldValue).Set(1) case "ovn-k8s-node-port": if fieldValue == "false" { ovnNodePortValue = 0 } case "ovn-bridge-mappings": + metricBridgeMappings.Reset() metricBridgeMappings.WithLabelValues(fieldValue).Set(1) } } diff --git a/go-controller/pkg/metrics/ovn_db.go b/go-controller/pkg/metrics/ovn_db.go index 8dec566031..2638fcb998 100644 --- a/go-controller/pkg/metrics/ovn_db.go +++ b/go-controller/pkg/metrics/ovn_db.go @@ -246,6 +246,8 @@ func ovnDBSizeMetricsUpdater(basePath, direction, database string) { if size, err := getOvnDBSizeViaPath(basePath, direction, database); err != nil { klog.Errorf("Failed to update OVN DB size metric: %v", err) } else { + // To update not only values but also labels for metrics, we use Reset() to delete previous labels+value + metricDBSize.Reset() metricDBSize.WithLabelValues(database).Set(float64(size)) } } @@ -291,6 +293,8 @@ func ovnDBMemoryMetricsUpdater(direction, database string) { // kvPair will be of the form monitors:2 fields := strings.Split(kvPair, ":") if value, err := strconv.ParseFloat(fields[1], 64); err == nil { + // To update not only values but also labels for metrics, we use Reset() to delete previous labels+value + metricOVNDBMonitor.Reset() metricOVNDBMonitor.WithLabelValues(database).Set(value) } else { klog.Errorf("Failed to parse the monitor's value %s to float64: err(%v)", @@ -300,6 +304,7 @@ func ovnDBMemoryMetricsUpdater(direction, database string) { // kvPair will be of the form sessions:2 fields := strings.Split(kvPair, ":") if value, err := strconv.ParseFloat(fields[1], 64); err == nil { + metricOVNDBSessions.Reset() metricOVNDBSessions.WithLabelValues(database).Set(value) } else { klog.Errorf("Failed to parse the sessions' value %s to float64: err(%v)", @@ -532,31 +537,47 @@ func ovnDBClusterStatusMetricsUpdater(direction, database string) { klog.Errorf(err.Error()) return } + // To update not only values but also labels for metrics, we use Reset() to delete previous labels+value + metricDBClusterCID.Reset() metricDBClusterCID.WithLabelValues(database, clusterStatus.cid).Set(1) + metricDBClusterSID.Reset() metricDBClusterSID.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(1) + metricDBClusterServerStatus.Reset() metricDBClusterServerStatus.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid, clusterStatus.status).Set(1) + metricDBClusterTerm.Reset() metricDBClusterTerm.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.term) + metricDBClusterServerRole.Reset() metricDBClusterServerRole.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid, clusterStatus.role).Set(1) + metricDBClusterServerVote.Reset() metricDBClusterServerVote.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid, clusterStatus.vote).Set(1) + metricDBClusterElectionTimer.Reset() metricDBClusterElectionTimer.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.electionTimer) + metricDBClusterLogIndexStart.Reset() metricDBClusterLogIndexStart.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.logIndexStart) + metricDBClusterLogIndexNext.Reset() metricDBClusterLogIndexNext.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.logIndexNext) + metricDBClusterLogNotCommitted.Reset() metricDBClusterLogNotCommitted.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.logNotCommitted) + metricDBClusterLogNotApplied.Reset() metricDBClusterLogNotApplied.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.logNotApplied) + metricDBClusterConnIn.Reset() metricDBClusterConnIn.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.connIn) + metricDBClusterConnOut.Reset() metricDBClusterConnOut.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.connOut) + metricDBClusterConnInErr.Reset() metricDBClusterConnInErr.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.connInErr) + metricDBClusterConnOutErr.Reset() metricDBClusterConnOutErr.WithLabelValues(database, clusterStatus.cid, clusterStatus.sid).Set(clusterStatus.connOutErr) } From 73eedd3e8e97d152b39921829c1ade48cfb96cfb Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Tue, 1 Mar 2022 21:15:59 +0100 Subject: [PATCH 05/25] CI: Retest only failed E2E jobs (#2824) * CI: Retest only failed E2E jobs Only retest failed E2E jobs out of the matrix. Do the same for DualStack and Upgrade tests. Test results will be stored inside github's cache and if a sub-job was marked as completed, it is skipped and its results from the run before are going to be displayed instead. Also cache master and PR builds. Signed-off-by: Andreas Karis --- .github/workflows/docker.yml | 2 +- .github/workflows/test.yml | 372 +++++++++++++++++++++++++++++++---- 2 files changed, 337 insertions(+), 37 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 6cdcda2020..de3097a272 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -7,7 +7,7 @@ on: paths: [ "dist/images/Dockerfile*"] env: - GO_VERSION: 1.16.3 + GO_VERSION: 1.17.6 jobs: build: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f962c04b5a..3e10a2a74a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,6 +17,23 @@ env: # Current Serial tests are not relevant for OVN PARALLEL: true + # This must be a directory + CI_IMAGE_CACHE: /tmp/image_cache/ + # This must be a directory + CI_RUN_LOG_CACHE: /tmp/run_logs/ + # This must be a file + CI_LAST_RUN_STATUS_CACHE: /tmp/last_run_status + + CI_IMAGE_MASTER_TAR: image-master.tar + CI_IMAGE_PR_TAR: image-pr.tar + CI_DIST_IMAGES_OUTPUT: dist/images/_output/ + + CI_LOGS_OVN_UPGRADE: logs_ovn_upgrade.txt + CI_LOGS_SHARD_CONFORMANCE: logs_shard_conformance.txt + CI_LOGS_GENERIC: logs.txt + CI_LOGS_DUAL_STACK: dual_stack_logs.txt + CI_LOGS_SINGLE_STACK: single_stack_logs.txt + jobs: # separate job for parallelism lint: @@ -37,18 +54,43 @@ jobs: name: Build-master runs-on: ubuntu-latest steps: + # Create a cache for the built master image + - name: Restore master image cache + id: image_cache_master + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_IMAGE_CACHE }} + key: ${{ github.run_id }}-image-cache-master + + - name: Check if master image build is needed + id: is_master_image_build_needed + continue-on-error: true + run: | + set -x + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR}.gz ]; then + mkdir -p ${CI_DIST_IMAGES_OUTPUT} + cp ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR}.gz ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_MASTER_TAR}.gz + gunzip ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_MASTER_TAR}.gz + echo "::set-output name=MASTER_IMAGE_RESTORED::true" + fi + + # only run the following steps if the master image was not found in the cache - name: Set up Go + if: steps.is_master_image_build_needed.outputs.MASTER_IMAGE_RESTORED != 'true' && success() uses: actions/setup-go@v2 with: go-version: ${{ env.GO_VERSION }} id: go - name: Check out code into the Go module directory - from master branch + if: steps.is_master_image_build_needed.outputs.MASTER_IMAGE_RESTORED != 'true' && success() uses: actions/checkout@v2 with: ref: master - name: Build - from master branch + if: steps.is_master_image_build_needed.outputs.MASTER_IMAGE_RESTORED != 'true' && success() run: | set -x pushd go-controller @@ -57,34 +99,76 @@ jobs: popd - name: Build docker image - from master branch + if: steps.is_master_image_build_needed.outputs.MASTER_IMAGE_RESTORED != 'true' && success() run: | pushd dist/images sudo cp -f ../../go-controller/_output/go/bin/ovn* . echo "ref: $(git rev-parse --symbolic-full-name HEAD) commit: $(git rev-parse HEAD)" > git_info docker build -t ovn-daemonset-f:dev -f Dockerfile.fedora . mkdir _output - docker save ovn-daemonset-f:dev > _output/image-master.tar + docker save ovn-daemonset-f:dev > _output/${CI_IMAGE_MASTER_TAR} popd + - name: Cache master image + if: steps.is_master_image_build_needed.outputs.MASTER_IMAGE_RESTORED != 'true' && success() + continue-on-error: true + run: | + set -x + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR} ]; then + rm -f ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR} + fi + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR}.gz ]; then + rm -f ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR}.gz + fi + mkdir -p ${CI_IMAGE_CACHE}/ + cp ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_MASTER_TAR} ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR} + gzip ${CI_IMAGE_CACHE}/${CI_IMAGE_MASTER_TAR} + + # run the following always if none of the steps before failed - uses: actions/upload-artifact@v2 with: name: test-image-master - path: dist/images/_output/image-master.tar + path: ${{ env.CI_DIST_IMAGES_OUTPUT }}/${{ env.CI_IMAGE_MASTER_TAR }} build-pr: name: Build-PR runs-on: ubuntu-latest steps: + # Create a cache for the build PR image + - name: Restore PR image cache + id: image_cache_pr + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_IMAGE_CACHE }} + key: ${{ github.run_id }}-image-cache-pr + + - name: Check if PR image build is needed + id: is_pr_image_build_needed + continue-on-error: true + run: | + set -x + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR}.gz ]; then + mkdir -p ${CI_DIST_IMAGES_OUTPUT} + cp ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR}.gz ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_PR_TAR}.gz + gunzip ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_PR_TAR}.gz + echo "::set-output name=PR_IMAGE_RESTORED::true" + fi + + # only run the following steps if the PR image was not found in the cache - name: Set up Go + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() uses: actions/setup-go@v2 with: go-version: ${{ env.GO_VERSION }} id: go - name: Check out code into the Go module directory - from current pr branch + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() uses: actions/checkout@v2 - name: Build and Test - from current pr branch + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() run: | set -x pushd go-controller @@ -96,28 +180,27 @@ jobs: popd - name: Build docker image - from current pr branch + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() run: | pushd dist/images sudo cp -f ../../go-controller/_output/go/bin/ovn* . echo "ref: $(git rev-parse --symbolic-full-name HEAD) commit: $(git rev-parse HEAD)" > git_info docker build -t ovn-daemonset-f:pr -f Dockerfile.fedora . mkdir _output - docker save ovn-daemonset-f:pr > _output/image-pr.tar + docker save ovn-daemonset-f:pr > _output/${CI_IMAGE_PR_TAR} popd - - uses: actions/upload-artifact@v2 - with: - name: test-image-pr - path: dist/images/_output/image-pr.tar - - name: Upload Junit Reports - if: always() + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && always() + continue-on-error: true uses: actions/upload-artifact@v2 with: name: junit-unit path: '**/_artifacts/**.xml' - name: Submit code coverage to Coveralls + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() + continue-on-error: true env: COVERALLS_TOKEN: ${{ secrets.GITHUB_TOKEN }} GO111MODULE: off @@ -133,6 +216,27 @@ jobs: gover goveralls -coverprofile=gover.coverprofile -service=github + - name: Cache PR image + if: steps.is_pr_image_build_needed.outputs.PR_IMAGE_RESTORED != 'true' && success() + continue-on-error: true + run: | + set -x + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR} ]; then + rm -f ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR} + fi + if [ -f ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR}.gz ]; then + rm -f ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR}.gz + fi + mkdir -p ${CI_IMAGE_CACHE}/ + cp ${CI_DIST_IMAGES_OUTPUT}/${CI_IMAGE_PR_TAR} ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR} + gzip ${CI_IMAGE_CACHE}/${CI_IMAGE_PR_TAR} + + # run the following if none of the previous steps failed + - uses: actions/upload-artifact@v2 + with: + name: test-image-pr + path: ${{ env.CI_DIST_IMAGES_OUTPUT }}/${{ env.CI_IMAGE_PR_TAR }} + ovn-upgrade-e2e: name: Upgrade OVN from Master to PR branch based image if: github.event_name != 'schedule' @@ -154,45 +258,83 @@ jobs: OVN_GATEWAY_MODE: "${{ matrix.gateway-mode }}" OVN_MULTICAST_ENABLE: "false" steps: + # This will write to key ${{ env.JOB_NAME }}-${{ github.run_id }} + - name: Initialize last run status cache + id: last_run_status_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_LAST_RUN_STATUS_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-last-run-status + + # The last run status comes from the run_cache file in the cache + # Verify all of the following steps. Only execute them if the cache does not + # contain: steps.last_run_status.outputs.STATUS != 'completed' and if none + # of the previous steps have failed + - name: Fetch last run status from file in cache + id: last_run_status + run: | + if [ -f ${CI_LAST_RUN_STATUS_CACHE} ]; then + cat ${CI_LAST_RUN_STATUS_CACHE} + fi + + # Create a cache for test results + - name: Create cache for run logs + id: run_log_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_RUN_LOG_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-run-logs + - name: Set up Go + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/setup-go@v1 with: go-version: ${{ env.GO_VERSION }} id: go - name: Set up environment + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | export GOPATH=$(go env GOPATH) echo "GOPATH=$GOPATH" >> $GITHUB_ENV echo "$GOPATH/bin" >> $GITHUB_PATH - name: Free up disk space + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: sudo eatmydata apt-get remove --auto-remove -y aspnetcore-* dotnet-* libmono-* mono-* msbuild php-* php7* ghc-* zulu-* - - uses: actions/download-artifact@v2 + - name: Download test-image-master + if: steps.last_run_status.outputs.STATUS != 'completed' && success() + uses: actions/download-artifact@v2 with: name: test-image-master - name: Disable ufw + if: steps.last_run_status.outputs.STATUS != 'completed' && success() # For IPv6 and Dualstack, ufw (Uncomplicated Firewall) should be disabled. # Not needed for KIND deployments, so just disable all the time. run: | sudo ufw disable - name: Load docker image + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | - docker load --input image-master.tar + docker load --input ${CI_IMAGE_MASTER_TAR} - name: Check out code into the Go module directory - from PR branch + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/checkout@v2 - name: kind setup + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | export OVN_IMAGE="ovn-daemonset-f:dev" make -C test install-kind - - name: Export logs - if: always() + - name: Export kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() run: | mkdir -p /tmp/kind/logs kind export logs --name ${KIND_CLUSTER_NAME} --loglevel=debug /tmp/kind/logs @@ -202,43 +344,70 @@ jobs: docker exec ovn-worker crictl images docker exec ovn-worker2 crictl images - - name: Upload logs - if: always() + - name: Upload kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-logs-${{ env.JOB_NAME }}-${{ github.run_id }} path: /tmp/kind/logs - - uses: actions/download-artifact@v2 + - name: Download test-image-pr + if: steps.last_run_status.outputs.STATUS != 'completed' && success() + uses: actions/download-artifact@v2 with: name: test-image-pr - name: Load docker image + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | - docker load --input image-pr.tar + docker load --input ${CI_IMAGE_PR_TAR} - name: ovn upgrade + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | + mkdir -p ${CI_RUN_LOG_CACHE} + exec > >(tee -a ${CI_RUN_LOG_CACHE}/${CI_LOGS_OVN_UPGRADE}) 2>&1 export OVN_IMAGE="ovn-daemonset-f:pr" make -C test upgrade-ovn - name: Run E2E shard-conformance + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | + mkdir -p ${CI_RUN_LOG_CACHE} + exec > >(tee -a ${CI_RUN_LOG_CACHE}/${CI_LOGS_SHARD_CONFORMANCE}) 2>&1 make -C test shard-conformance - - name: Export logs - if: always() + - name: Export kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() run: | mkdir -p /tmp/kind/logs-kind-pr-branch kind export logs --name ${KIND_CLUSTER_NAME} --loglevel=debug /tmp/kind/logs-kind-pr-branch - - name: Upload logs - if: always() + - name: Upload kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-logs-${{ env.JOB_NAME }}-${{ github.run_id }}-after-upgrade path: /tmp/kind/logs-kind-pr-branch + # The following steps will run if the job is marked completed and no step failed + - name: Display run logs from successful tests + if: steps.last_run_status.outputs.STATUS == 'completed' && success() + continue-on-error: true + run: | + if [ -f ${CI_RUN_LOG_CACHE}/${CI_LOGS_OVN_UPGRADE} ]; then + cat ${CI_RUN_LOG_CACHE}/${CI_LOGS_OVN_UPGRADE} + fi + if [ -f ${CI_RUN_LOG_CACHE}/${CI_LOGS_SHARD_CONFORMANCE} ]; then + cat ${CI_RUN_LOG_CACHE}/${CI_LOGS_SHARD_CONFORMANCE} + fi + + # This will set the name=STATUS to 'completed' if none of the above steps + # failed + - name: Set last run status to completed + run: | + echo '::set-output name=STATUS::completed' > ${CI_LAST_RUN_STATUS_CACHE} + e2e: name: e2e if: github.event_name != 'schedule' @@ -294,20 +463,52 @@ jobs: KIND_IPV4_SUPPORT: "${{ matrix.ipfamily == 'IPv4' || matrix.ipfamily == 'dualstack' }}" KIND_IPV6_SUPPORT: "${{ matrix.ipfamily == 'IPv6' || matrix.ipfamily == 'dualstack' }}" steps: + # This will write to key ${{ env.JOB_NAME }}-${{ github.run_id }} + - name: Initialize last run status cache + id: last_run_status_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_LAST_RUN_STATUS_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-last-run-status + + # The last run status comes from the run_cache file in the cache + # Verify all of the following steps. Only execute them if the cache does not + # contain: steps.last_run_status.outputs.STATUS != 'completed' and if none + # of the previous steps have failed + - name: Fetch last run status from file in cache + id: last_run_status + run: | + if [ -f ${CI_LAST_RUN_STATUS_CACHE} ]; then + cat ${CI_LAST_RUN_STATUS_CACHE} + fi + + # Create a cache for test results + - name: Create cache for run logs + id: run_log_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_RUN_LOG_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-run-logs - name: Free up disk space + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: sudo eatmydata apt-get remove --auto-remove -y aspnetcore-* dotnet-* libmono-* mono-* msbuild php-* php7* ghc-* zulu-* - name: Set up Go + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/setup-go@v2 with: go-version: ${{ env.GO_VERSION }} id: go - name: Check out code into the Go module directory + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/checkout@v2 - name: Set up environment + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | export GOPATH=$(go env GOPATH) echo "GOPATH=$GOPATH" >> $GITHUB_ENV @@ -316,48 +517,78 @@ jobs: echo OVN_TEST_EX_GW_NETWORK=kindexgw >> $GITHUB_ENV echo OVN_ENABLE_EX_GW_NETWORK_BRIDGE=true >> $GITHUB_ENV fi + - name: Disable ufw + if: steps.last_run_status.outputs.STATUS != 'completed' && success() # For IPv6 and Dualstack, ufw (Uncomplicated Firewall) should be disabled. # Not needed for KIND deployments, so just disable all the time. run: | sudo ufw disable - - uses: actions/download-artifact@v2 + + - name: Download test-image-pr + if: steps.last_run_status.outputs.STATUS != 'completed' && success() + uses: actions/download-artifact@v2 with: name: test-image-pr - name: Load docker image + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | - docker load --input image-pr.tar + docker load --input ${CI_IMAGE_PR_TAR} + - name: kind setup + if: steps.last_run_status.outputs.STATUS != 'completed' && success() timeout-minutes: 30 run: | export OVN_IMAGE="ovn-daemonset-f:pr" make -C test install-kind + - name: Run Tests + if: steps.last_run_status.outputs.STATUS != 'completed' && success() # e2e tests take ~60 minutes normally, 90 should be more than enough # set 2 1/2 hours for control-plane tests as these might take a while timeout-minutes: ${{ matrix.target == 'control-plane' && 150 || 90 }} run: | + mkdir -p ${CI_RUN_LOG_CACHE} + exec > >(tee -a ${CI_RUN_LOG_CACHE}/${CI_LOGS_GENERIC}) 2>&1 make -C test ${{ matrix.target }} + + # The following steps will always run unless the job is marked as completed - name: Upload Junit Reports - if: always() + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-junit-${{ env.JOB_NAME }}-${{ github.run_id }} path: './test/_artifacts/*.xml' - - name: Export logs - if: always() + - name: Export kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() run: | mkdir -p /tmp/kind/logs kind export logs --name ${KIND_CLUSTER_NAME} --loglevel=debug /tmp/kind/logs - - name: Upload logs - if: always() + + - name: Upload kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-logs-${{ env.JOB_NAME }}-${{ github.run_id }} path: /tmp/kind/logs + # The following steps will run if the job is marked completed and no step failed + - name: Display run logs from successful tests + if: steps.last_run_status.outputs.STATUS == 'completed' && success() + continue-on-error: true + run: | + if [ -f ${CI_RUN_LOG_CACHE}/${CI_LOGS_GENERIC} ]; then + cat ${CI_RUN_LOG_CACHE}/${CI_LOGS_GENERIC} + fi + + # This will set the name=STATUS to 'completed' if none of the above steps + # failed + - name: Set last run status to completed + run: | + echo '::set-output name=STATUS::completed' > ${CI_LAST_RUN_STATUS_CACHE} + e2e-dual-conversion: name: e2e-dual-conversion if: github.event_name != 'schedule' @@ -377,67 +608,136 @@ jobs: OVN_GATEWAY_MODE: "${{ matrix.gateway-mode }}" OVN_MULTICAST_ENABLE: "false" steps: + # This will write to key ${{ env.JOB_NAME }}-${{ github.run_id }} + - name: Initialize last run status cache + id: last_run_status_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_LAST_RUN_STATUS_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-last-run-status + + # The last run status comes from the run_cache file in the cache + # Verify all of the following steps. Only execute them if the cache does not + # contain: steps.last_run_status.outputs.STATUS != 'completed' and if none + # of the previous steps have failed + - name: Fetch last run status from file in cache + id: last_run_status + run: | + if [ -f ${CI_LAST_RUN_STATUS_CACHE} ]; then + cat ${CI_LAST_RUN_STATUS_CACHE} + fi + + # Create a cache for test results + - name: Create cache for run logs + id: run_log_cache + uses: actions/cache@v2 + with: + path: | + ${{ env.CI_RUN_LOG_CACHE }} + key: ${{ env.JOB_NAME }}-${{ github.run_id }}-run-logs - name: Set up Go + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/setup-go@v2 with: go-version: ${{ env.GO_VERSION }} id: go - name: Check out code into the Go module directory + if: steps.last_run_status.outputs.STATUS != 'completed' && success() uses: actions/checkout@v2 - name: Set up environment + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | export GOPATH=$(go env GOPATH) echo "GOPATH=$GOPATH" >> $GITHUB_ENV echo "$GOPATH/bin" >> $GITHUB_PATH + - name: Disable ufw + if: steps.last_run_status.outputs.STATUS != 'completed' && success() # For IPv6 and Dualstack, ufw (Uncomplicated Firewall) should be disabled. # Not needed for KIND deployments, so just disable all the time. run: | sudo ufw disable - - uses: actions/download-artifact@v2 + + - name: Download test-image-pr + if: steps.last_run_status.outputs.STATUS != 'completed' && success() + uses: actions/download-artifact@v2 with: name: test-image-pr - name: Load docker image + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | - docker load --input image-pr.tar + docker load --input ${CI_IMAGE_PR_TAR} + - name: kind IPv4 setup + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | export OVN_IMAGE="ovn-daemonset-f:pr" make -C test install-kind + - name: Run Single-Stack Tests + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | + mkdir -p ${CI_RUN_LOG_CACHE} + exec > >(tee -a ${CI_RUN_LOG_CACHE}/${CI_LOGS_SINGLE_STACK}) 2>&1 make -C test shard-test WHAT="Networking Granular Checks" + - name: Convert IPv4 cluster to Dual Stack + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | ./contrib/kind-dual-stack-conversion.sh + - name: Run Dual-Stack Tests + if: steps.last_run_status.outputs.STATUS != 'completed' && success() run: | + mkdir -p ${CI_RUN_LOG_CACHE} + exec > >(tee -a ${CI_RUN_LOG_CACHE}/${CI_LOGS_DUAL_STACK}) 2>&1 KIND_IPV4_SUPPORT="true" KIND_IPV6_SUPPORT="true" make -C test shard-test WHAT="Networking Granular Checks\|DualStack" + - name: Upload Junit Reports - if: always() + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-junit-${{ env.JOB_NAME }}-${{ github.run_id }} path: './test/_artifacts/*.xml' - - name: Export logs - if: always() + - name: Export kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() run: | mkdir -p /tmp/kind/logs kind export logs --name ${KIND_CLUSTER_NAME} --loglevel=debug /tmp/kind/logs - - name: Upload logs - if: always() + + - name: Upload kind logs + if: steps.last_run_status.outputs.STATUS != 'completed' && always() uses: actions/upload-artifact@v2 with: name: kind-logs-${{ env.JOB_NAME }}-${{ github.run_id }} path: /tmp/kind/logs + # The following steps will run if the job is marked completed and no step failed + - name: Display run logs from successful tests + if: steps.last_run_status.outputs.STATUS == 'completed' && success() + continue-on-error: true + run: | + if [ -f ${CI_RUN_LOG_CACHE}/${CI_LOGS_SINGLE_STACK} ]; then + cat ${CI_RUN_LOG_CACHE}/${CI_LOGS_SINGLE_STACK} + fi + if [ -f ${CI_RUN_LOG_CACHE}/${CI_LOGS_DUAL_STACK} ]; then + cat ${CI_RUN_LOG_CACHE}/${CI_LOGS_DUAL_STACK} + fi + + # This will set the name=STATUS to 'completed' if none of the above steps + # failed + - name: Set last run status to completed + run: | + echo '::set-output name=STATUS::completed' > ${CI_LAST_RUN_STATUS_CACHE} + e2e-periodic: name: e2e-periodic if: github.event_name == 'schedule' @@ -487,7 +787,7 @@ jobs: name: test-image-pr - name: Load docker image run: | - docker load --input image-pr.tar + docker load --input ${CI_IMAGE_PR_TAR} - name: kind setup run: | export OVN_IMAGE="ovn-daemonset-f:pr" From c013af1045a3ca65d1073e25c3a6133a15d09dcb Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 15 Nov 2021 23:12:40 -0600 Subject: [PATCH 06/25] egressgw: harmonize all three places pod GW routes are deleted It's all the same code, move some things around so they all look the same in preparation for the next step. Signed-off-by: Dan Williams --- go-controller/pkg/ovn/egressgw.go | 48 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/go-controller/pkg/ovn/egressgw.go b/go-controller/pkg/ovn/egressgw.go index 2fd4a695f8..6a7c8eb985 100644 --- a/go-controller/pkg/ovn/egressgw.go +++ b/go-controller/pkg/ovn/egressgw.go @@ -386,11 +386,11 @@ func (oc *Controller) deletePodGWRoutesForNamespace(pod *kapi.Pod, namespace str continue } for podIP, route := range routeInfo.podExternalRoutes { + mask := GetIPFullMask(podIP) for routeGwIP, gr := range route { if gwIP.String() != routeGwIP { continue } - mask := GetIPFullMask(podIP) node := util.GetWorkerFromGatewayRouter(gr) portPrefix, err := oc.extSwitchPrefix(node) if err != nil { @@ -403,20 +403,17 @@ func (oc *Controller) deletePodGWRoutesForNamespace(pod *kapi.Pod, namespace str pod, gr, gwIP.String(), err) klog.Error(err) } else { + delete(routeInfo.podExternalRoutes[podIP], gwIP.String()) klog.V(5).Infof("ECMP route deleted for pod: %s, on gr: %s, to gw: %s", pod, gr, gwIP.String()) + } - delete(routeInfo.podExternalRoutes[podIP], gwIP.String()) - // clean up if there are no more routes for this podIP - if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { - // TODO (trozet): use the go bindings here and batch commands - // delete the ovn_cluster_router policy if the pod has no more exgws to revert back to normal - // default gw behavior - if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { - klog.Error(err) - } + if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { + if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { + klog.Error(err) } } + oc.cleanUpBFDEntry(gwIP.String(), gr, portPrefix) } } @@ -436,28 +433,30 @@ func (oc *Controller) deleteGWRoutesForNamespace(namespace string) { continue } for podIP, gwToGr := range routeInfo.podExternalRoutes { + mask := GetIPFullMask(podIP) for gw, gr := range gwToGr { if utilnet.IsIPv6String(gw) != utilnet.IsIPv6String(podIP) { continue } - mask := GetIPFullMask(podIP) node := util.GetWorkerFromGatewayRouter(gr) + portPrefix, err := oc.extSwitchPrefix(node) + if err != nil { + klog.Infof("Failed to find ext switch prefix for %s %v", node, err) + continue + } + if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gw, gr); err != nil { klog.Errorf("Unable to delete src-ip route to GR router, err:%v", err) } else { delete(routeInfo.podExternalRoutes[podIP], gw) } + if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { klog.Error(err) } } - portPrefix, err := oc.extSwitchPrefix(node) - if err != nil { - klog.Infof("Failed to find ext switch prefix for %s %v", node, err) - continue - } oc.cleanUpBFDEntry(gw, gr, portPrefix) } } @@ -474,13 +473,13 @@ func (oc *Controller) deleteGWRoutesForPod(name ktypes.NamespacedName, podIPNets defer routeInfo.Unlock() for _, podIPNet := range podIPNets { - pod := podIPNet.IP.String() - if gwToGr, ok := routeInfo.podExternalRoutes[pod]; ok { + podIP := podIPNet.IP.String() + mask := GetIPFullMask(podIP) + if gwToGr, ok := routeInfo.podExternalRoutes[podIP]; ok { if len(gwToGr) == 0 { - delete(routeInfo.podExternalRoutes, pod) + delete(routeInfo.podExternalRoutes, podIP) continue } - mask := GetIPFullMask(pod) for gw, gr := range gwToGr { node := util.GetWorkerFromGatewayRouter(gr) portPrefix, err := oc.extSwitchPrefix(node) @@ -488,19 +487,22 @@ func (oc *Controller) deleteGWRoutesForPod(name ktypes.NamespacedName, podIPNets klog.Infof("Failed to find ext switch prefix for %s %v", node, err) continue } - if err := oc.deleteLogicalRouterStaticRoute(pod, mask, gw, gr); err != nil { + + if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gw, gr); err != nil { klog.Errorf("Unable to delete ECMP route for pod: %s to GR %s, GW: %s, err:%v", name, gr, gw, err) } else { - delete(routeInfo.podExternalRoutes[pod], gw) + delete(routeInfo.podExternalRoutes[podIP], gw) klog.V(5).Infof("ECMP route deleted for pod: %s, on gr: %s, to gw: %s", name, gr, gw) } - if entry := routeInfo.podExternalRoutes[pod]; len(entry) == 0 { + + if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { if err := oc.delHybridRoutePolicyForPod(podIPNet.IP, node); err != nil { klog.Error(err) } } + oc.cleanUpBFDEntry(gw, gr, portPrefix) } } From 0edbe3a4de5284bb0d4b9f7c2dc665de07023539 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 15 Nov 2021 23:21:31 -0600 Subject: [PATCH 07/25] egressgw: pull pod gateway route deletion into a function Signed-off-by: Dan Williams --- go-controller/pkg/ovn/egressgw.go | 126 ++++++++++++------------------ 1 file changed, 50 insertions(+), 76 deletions(-) diff --git a/go-controller/pkg/ovn/egressgw.go b/go-controller/pkg/ovn/egressgw.go index 6a7c8eb985..ed07e69825 100644 --- a/go-controller/pkg/ovn/egressgw.go +++ b/go-controller/pkg/ovn/egressgw.go @@ -344,6 +344,36 @@ func (oc *Controller) deleteLogicalRouterStaticRoute(podIP, mask, gw, gr string) return nil } +// deletePodGWRoute deletes all associated gateway routing resources for one +// pod gateway route +func (oc *Controller) deletePodGWRoute(routeInfo *externalRouteInfo, podIP, gw, gr string) error { + if utilnet.IsIPv6String(gw) != utilnet.IsIPv6String(podIP) { + return nil + } + + mask := GetIPFullMask(podIP) + if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gw, gr); err != nil { + return fmt.Errorf("unable to delete pod %s ECMP route to GR %s, GW: %s: %w", + routeInfo.podName, gr, gw, err) + } + + klog.V(5).Infof("ECMP route deleted for pod: %s, on gr: %s, to gw: %s", + routeInfo.podName, gr, gw) + + node := util.GetWorkerFromGatewayRouter(gr) + if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { + if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { + return err + } + } + + portPrefix, err := oc.extSwitchPrefix(node) + if err != nil { + return err + } + return oc.cleanUpBFDEntry(gw, gr, portPrefix) +} + // deletePodExternalGW detects if a given pod is acting as an external GW and removes all routes in all namespaces // associated with that pod func (oc *Controller) deletePodExternalGW(pod *kapi.Pod) { @@ -386,35 +416,14 @@ func (oc *Controller) deletePodGWRoutesForNamespace(pod *kapi.Pod, namespace str continue } for podIP, route := range routeInfo.podExternalRoutes { - mask := GetIPFullMask(podIP) for routeGwIP, gr := range route { - if gwIP.String() != routeGwIP { - continue - } - node := util.GetWorkerFromGatewayRouter(gr) - portPrefix, err := oc.extSwitchPrefix(node) - if err != nil { - klog.Infof("Failed to find ext switch prefix for %s %v", node, err) - continue - } - - if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gwIP.String(), gr); err != nil { - klog.Errorf("Unable to delete pod %s route to GR %s, GW: %s, err:%v", - pod, gr, gwIP.String(), err) - klog.Error(err) - } else { - delete(routeInfo.podExternalRoutes[podIP], gwIP.String()) - klog.V(5).Infof("ECMP route deleted for pod: %s, on gr: %s, to gw: %s", pod, - gr, gwIP.String()) - } - - if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { - if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { - klog.Error(err) + if gwIP.String() == routeGwIP { + if err := oc.deletePodGWRoute(routeInfo, podIP, gwIP.String(), gr); err != nil { + klog.Errorf(err.Error()) + continue } + delete(route, routeGwIP) } - - oc.cleanUpBFDEntry(gwIP.String(), gr, portPrefix) } } routeInfo.Unlock() @@ -433,31 +442,12 @@ func (oc *Controller) deleteGWRoutesForNamespace(namespace string) { continue } for podIP, gwToGr := range routeInfo.podExternalRoutes { - mask := GetIPFullMask(podIP) for gw, gr := range gwToGr { - if utilnet.IsIPv6String(gw) != utilnet.IsIPv6String(podIP) { + if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { + klog.Errorf(err.Error()) continue } - node := util.GetWorkerFromGatewayRouter(gr) - portPrefix, err := oc.extSwitchPrefix(node) - if err != nil { - klog.Infof("Failed to find ext switch prefix for %s %v", node, err) - continue - } - - if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gw, gr); err != nil { - klog.Errorf("Unable to delete src-ip route to GR router, err:%v", err) - } else { - delete(routeInfo.podExternalRoutes[podIP], gw) - } - - if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { - if err := oc.delHybridRoutePolicyForPod(net.ParseIP(podIP), node); err != nil { - klog.Error(err) - } - } - - oc.cleanUpBFDEntry(gw, gr, portPrefix) + delete(gwToGr, gw) } } routeInfo.Unlock() @@ -474,36 +464,17 @@ func (oc *Controller) deleteGWRoutesForPod(name ktypes.NamespacedName, podIPNets for _, podIPNet := range podIPNets { podIP := podIPNet.IP.String() - mask := GetIPFullMask(podIP) if gwToGr, ok := routeInfo.podExternalRoutes[podIP]; ok { if len(gwToGr) == 0 { delete(routeInfo.podExternalRoutes, podIP) continue } for gw, gr := range gwToGr { - node := util.GetWorkerFromGatewayRouter(gr) - portPrefix, err := oc.extSwitchPrefix(node) - if err != nil { - klog.Infof("Failed to find ext switch prefix for %s %v", node, err) + if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { + klog.Errorf(err.Error()) continue } - - if err := oc.deleteLogicalRouterStaticRoute(podIP, mask, gw, gr); err != nil { - klog.Errorf("Unable to delete ECMP route for pod: %s to GR %s, GW: %s, err:%v", - name, gr, gw, err) - } else { - delete(routeInfo.podExternalRoutes[podIP], gw) - klog.V(5).Infof("ECMP route deleted for pod: %s, on gr: %s, to gw: %s", name, - gr, gw) - } - - if entry := routeInfo.podExternalRoutes[podIP]; len(entry) == 0 { - if err := oc.delHybridRoutePolicyForPod(podIPNet.IP, node); err != nil { - klog.Error(err) - } - } - - oc.cleanUpBFDEntry(gw, gr, portPrefix) + delete(gwToGr, gw) } } } @@ -924,7 +895,7 @@ func (oc *Controller) delAllLegacyHybridRoutePolicies() error { // cleanUpBFDEntry checks if the BFD table entry related to the associated // gw router / port / gateway ip is referenced by other routing rules, and if // not removes the entry to avoid having dangling BFD entries. -func (oc *Controller) cleanUpBFDEntry(gatewayIP, gatewayRouter, prefix string) { +func (oc *Controller) cleanUpBFDEntry(gatewayIP, gatewayRouter, prefix string) error { portName := prefix + types.GWRouterToExtSwitchPrefix + gatewayRouter ctx, cancel := context.WithTimeout(context.Background(), types.OVSDBTimeout) @@ -934,12 +905,11 @@ func (oc *Controller) cleanUpBFDEntry(gatewayIP, gatewayRouter, prefix string) { return lrsr.OutputPort != nil && *lrsr.OutputPort == portName && lrsr.Nexthop == gatewayIP && lrsr.BFD != nil && *lrsr.BFD != "" }).List(ctx, &logicalRouterStaticRouteRes) if err != nil { - klog.Errorf("cleanUpBFDEntry: failed to list routes for %s, err: %v", portName, err) - return + return fmt.Errorf("cleanUpBFDEntry failed to list routes for %s: %w", portName, err) } if len(logicalRouterStaticRouteRes) > 0 { - return + return nil } opModels := []libovsdbops.OperationModel{ @@ -951,8 +921,10 @@ func (oc *Controller) cleanUpBFDEntry(gatewayIP, gatewayRouter, prefix string) { }, } if err := oc.modelClient.Delete(opModels...); err != nil { - klog.Errorf("Failed to delete BFD, err: %v", err) + return fmt.Errorf("failed to delete BFD: %w", err) } + + return nil } // extSwitchPrefix returns the prefix of the external switch to use for @@ -1092,7 +1064,9 @@ func (oc *Controller) cleanExGwECMPRoutes() { klog.Errorf("Cannot sync exgw bfd: %+v, unable to determine exgw switch prefix: %v", ovnRoute, err) } else { - oc.cleanUpBFDEntry(ovnRoute.nextHop, ovnRoute.router, prefix) + if err := oc.cleanUpBFDEntry(ovnRoute.nextHop, ovnRoute.router, prefix); err != nil { + klog.Errorf(err.Error()) + } } } else { From a8e94ca661fdce746081b3fd1c8c31a228524e6e Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 16 Nov 2021 11:38:47 -0600 Subject: [PATCH 08/25] egressgw: consolidate route deletion functions Signed-off-by: Dan Williams --- go-controller/pkg/ovn/egressgw.go | 72 +++++++++++++----------------- go-controller/pkg/ovn/namespace.go | 4 +- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/go-controller/pkg/ovn/egressgw.go b/go-controller/pkg/ovn/egressgw.go index ed07e69825..e4ab8ae8a2 100644 --- a/go-controller/pkg/ovn/egressgw.go +++ b/go-controller/pkg/ovn/egressgw.go @@ -28,6 +28,7 @@ import ( kapi "k8s.io/api/core/v1" ktypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" ) @@ -406,48 +407,33 @@ func (oc *Controller) deletePodGWRoutesForNamespace(pod *kapi.Pod, namespace str return } + gws := sets.NewString() for _, gwIP := range foundGws.gws { - // check for previously configured pod routes - routeInfos := oc.getRouteInfosForNamespace(namespace) - for _, routeInfo := range routeInfos { - routeInfo.Lock() - if routeInfo.deleted { - routeInfo.Unlock() - continue - } - for podIP, route := range routeInfo.podExternalRoutes { - for routeGwIP, gr := range route { - if gwIP.String() == routeGwIP { - if err := oc.deletePodGWRoute(routeInfo, podIP, gwIP.String(), gr); err != nil { - klog.Errorf(err.Error()) - continue - } - delete(route, routeGwIP) - } - } - } - routeInfo.Unlock() - } + gws.Insert(gwIP.String()) } + oc.deleteGWRoutesForNamespace(namespace, gws) } -// deleteGwRoutesForNamespace handles deleting all routes to gateways for a pod on a specific GR -func (oc *Controller) deleteGWRoutesForNamespace(namespace string) { - // TODO(trozet): batch all of these with ebay bindings - routeInfos := oc.getRouteInfosForNamespace(namespace) - for _, routeInfo := range routeInfos { +// deleteGwRoutesForNamespace handles deleting routes to gateways for a pod on a specific GR. +// If a set of gateways is given, only routes for that gateway are deleted. If no gateways +// are given, all routes for the namespace are deleted. +func (oc *Controller) deleteGWRoutesForNamespace(namespace string, matchGWs sets.String) { + deleteAll := (matchGWs == nil || matchGWs.Len() == 0) + for _, routeInfo := range oc.getRouteInfosForNamespace(namespace) { routeInfo.Lock() if routeInfo.deleted { routeInfo.Unlock() continue } - for podIP, gwToGr := range routeInfo.podExternalRoutes { - for gw, gr := range gwToGr { - if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { - klog.Errorf(err.Error()) - continue + for podIP, routes := range routeInfo.podExternalRoutes { + for gw, gr := range routes { + if deleteAll || matchGWs.Has(gw) { + if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { + klog.Errorf(err.Error()) + continue + } + delete(routes, gw) } - delete(gwToGr, gw) } } routeInfo.Unlock() @@ -464,18 +450,20 @@ func (oc *Controller) deleteGWRoutesForPod(name ktypes.NamespacedName, podIPNets for _, podIPNet := range podIPNets { podIP := podIPNet.IP.String() - if gwToGr, ok := routeInfo.podExternalRoutes[podIP]; ok { - if len(gwToGr) == 0 { - delete(routeInfo.podExternalRoutes, podIP) + routes, ok := routeInfo.podExternalRoutes[podIP] + if !ok { + continue + } + if len(routes) == 0 { + delete(routeInfo.podExternalRoutes, podIP) + continue + } + for gw, gr := range routes { + if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { + klog.Errorf(err.Error()) continue } - for gw, gr := range gwToGr { - if err := oc.deletePodGWRoute(routeInfo, podIP, gw, gr); err != nil { - klog.Errorf(err.Error()) - continue - } - delete(gwToGr, gw) - } + delete(routes, gw) } } } diff --git a/go-controller/pkg/ovn/namespace.go b/go-controller/pkg/ovn/namespace.go index 80b307f5f5..51c1cd230f 100644 --- a/go-controller/pkg/ovn/namespace.go +++ b/go-controller/pkg/ovn/namespace.go @@ -305,7 +305,7 @@ func (oc *Controller) updateNamespace(old, newer *kapi.Namespace) { } } } else { - oc.deleteGWRoutesForNamespace(old.Name) + oc.deleteGWRoutesForNamespace(old.Name, nil) nsInfo.routingExternalGWs = gatewayInfo{} } exGateways, err := parseRoutingExternalGWAnnotation(gwAnnotation) @@ -378,7 +378,7 @@ func (oc *Controller) deleteNamespace(ns *kapi.Namespace) { delete(nsInfo.networkPolicies, np.name) } } - oc.deleteGWRoutesForNamespace(ns.Name) + oc.deleteGWRoutesForNamespace(ns.Name, nil) oc.multicastDeleteNamespace(ns, nsInfo) } From ef85604317e22e17f713b1e964a9aac91534153e Mon Sep 17 00:00:00 2001 From: astoycos Date: Tue, 1 Mar 2022 16:17:32 -0500 Subject: [PATCH 09/25] Add the following capibilities to kind.sh - To run the setup script from a container - To explicitly supply a kubeconfig - To explicitly name the cluster - To provide a custom service DNSdomain - To itegrate a locally running Docker Registry Remove bits that manually configure apiserver-ip for the kind cluster since it could prove to be a security venerability in CI. Instead always default to the node's local loopback address as suggested in KIND documentation. Signed-off-by: astoycos --- contrib/kind.sh | 186 +++++++++++++++++++++++++++------------ contrib/kind.yaml.j2 | 14 ++- dist/images/daemonset.sh | 46 +++++++--- 3 files changed, 175 insertions(+), 71 deletions(-) diff --git a/contrib/kind.sh b/contrib/kind.sh index 1e79e945c6..d855ef1425 100755 --- a/contrib/kind.sh +++ b/contrib/kind.sh @@ -1,9 +1,8 @@ #!/usr/bin/env bash +# Returns the full directory name of the script DIR="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" -export KUBECONFIG=${HOME}/admin.conf - run_kubectl() { local retries=0 local attempts=10 @@ -33,7 +32,7 @@ delete() { } usage() { - echo "usage: kind.sh [[[-cf |--config-file ] [-kt|keep-taint] [-ha|--ha-enabled]" + echo "usage: kind.sh [[[-cf |--config-file ] [-kt|--keep-taint] [-ha|--ha-enabled]" echo " [-ho |--hybrid-enabled] [-ii|--install-ingress] [-n4|--no-ipv4]" echo " [-i6 |--ipv6] [-wk|--num-workers ] [-ds|--disable-snat-multiple-gws]" echo " [-dp |--disable-pkt-mtu-check]" @@ -46,7 +45,11 @@ usage() { echo " [-nbl|--ovn-loglevel-nb ] [-sbl|--ovn-loglevel-sb ]" echo " [-cl |--ovn-loglevel-controller ] [-dl|--ovn-loglevel-nbctld ]" echo " [-ep |--experimental-provider ] |" - echo " [-eb |--egress-gw-separate-bridge]" + echo " [-eb |--egress-gw-separate-bridge] |" + echo " [-lr |--local-kind-registry |" + echo " [-dd |--dns-domain |" + echo " [-ric | --run-in-container |" + echo " [-cn | --cluster-name |" echo " [-h]]" echo "" echo "-cf | --config-file Name of the KIND J2 configuration file." @@ -86,6 +89,10 @@ usage() { echo "-dl | --ovn-loglevel-nbctld Log config for nbctl daemon DEFAULT: '-vconsole:info'." echo "-ep | --experimental-provider Use an experimental OCI provider such as podman, instead of docker. DEFAULT: Disabled." echo "-eb | --egress-gw-separate-bridge The external gateway traffic uses a separate bridge." + echo "-lr | --local-kind-registry Configure kind to use a local docker registry rather than manually loading images" + echo "-dd | --dns-domain Configure a custom dnsDomain for k8s services, Defaults to 'cluster.local'" + echo "-cn | --cluster-name Configure the kind cluster's name" + echo "-ric | --run-in-container Configure the script to be run from a docker container, allowing it to still communcate with the kind controlplane" echo "--delete Delete current cluster" echo "" } @@ -206,6 +213,19 @@ parse_args() { ;; -hns | --host-network-namespace ) OVN_HOST_NETWORK_NAMESPACE=$1 ;; + -lr | --local-kind-registry ) KIND_LOCAL_REGISTRY=true + ;; + -dd | --dns-domain ) shift + KIND_DNS_DOMAIN=$1 + ;; + -cn | --cluster-name ) shift + KIND_CLUSTER_NAME=$1 + ;; + -kc | --kubeconfig ) shift + KUBECONFIG=$1 + ;; + -ric | --run-in-container ) RUN_IN_CONTAINER=true + ;; --delete ) delete exit ;; @@ -222,8 +242,14 @@ parse_args() { print_params() { echo "Using these parameters to install KIND" echo "" + echo "KUBECONFIG = $KUBECONFIG" + echo "MANIFEST_OUTPUT_DIR = $MANIFEST_OUTPUT_DIR" echo "KIND_INSTALL_INGRESS = $KIND_INSTALL_INGRESS" echo "OVN_HA = $OVN_HA" + echo "RUN_IN_CONTAINER = $RUN_IN_CONTAINER" + echo "KIND_CLUSTER_NAME = $KIND_CLUSTER_NAME" + echo "KIND_LOCAL_REGISTRY = $KIND_LOCAL_REGISTRY" + echo "KIND_DNS_DOMAIN = $KIND_DNS_DOMAIN" echo "KIND_CONFIG_FILE = $KIND_CONFIG" echo "KIND_REMOVE_TAINT = $KIND_REMOVE_TAINT" echo "KIND_IPV4_SUPPORT = $KIND_IPV4_SUPPORT" @@ -260,16 +286,35 @@ print_params() { command_exists() { cmd="$1" - which ${cmd} >/dev/null 2>&1 + command -v ${cmd} >/dev/null 2>&1 +} + +install_j2_renderer() { + # ensure j2 renderer installed + pip install wheel --user + pip freeze | grep j2cli || pip install j2cli[yaml] --user + export PATH=~/.local/bin:$PATH } check_dependencies() { - for cmd in pip jq kind ; do - if ! command_exists ${cmd} ; then - echo "Dependency not met: Command not found '${cmd}'" - exit 1 + if ! command_exists kind ; then + echo "Dependency not met: Command not found 'kind'" + exit 1 + fi + + if ! command_exists jq ; then + echo "Dependency not met: Command not found 'jq'" + exit 1 + fi + + if ! command_exists j2 ; then + if ! command_exists pip ; then + echo "Dependency not met: 'j2' not installed and cannot install with 'pip'" + exit 1 fi - done + echo "'j2' not found, installing with 'pip'" + install_j2_renderer + fi if ! command_exists docker && ! command_exists podman; then echo "Dependency not met: Neither docker nor podman found" @@ -277,21 +322,26 @@ check_dependencies() { fi } -install_j2_renderer() { - # ensure j2 renderer installed - pip install wheel --user - pip freeze | grep j2cli || pip install j2cli[yaml] --user - export PATH=~/.local/bin:$PATH -} - set_default_params() { # Set default values + # Used for multi cluster setups KIND_CLUSTER_NAME=${KIND_CLUSTER_NAME:-ovn} + # Setup KUBECONFIG patch based on cluster-name + export KUBECONFIG=${KUBECONFIG:-${HOME}/${KIND_CLUSTER_NAME}.conf} + # Scrub any existing kubeconfigs at the path + rm -f ${KUBECONFIG} + MANIFEST_OUTPUT_DIR=${MANIFEST_OUTPUT_DIR:-${DIR}/../dist/yaml} + if [ ${KIND_CLUSTER_NAME} != "ovn" ]; then + MANIFEST_OUTPUT_DIR="${DIR}/../dist/yaml/${KIND_CLUSTER_NAME}" + fi + RUN_IN_CONTAINER=${RUN_IN_CONTAINER:-false} KIND_IMAGE=${KIND_IMAGE:-kindest/node} K8S_VERSION=${K8S_VERSION:-v1.23.3} OVN_GATEWAY_MODE=${OVN_GATEWAY_MODE:-shared} KIND_INSTALL_INGRESS=${KIND_INSTALL_INGRESS:-false} OVN_HA=${OVN_HA:-false} + KIND_LOCAL_REGISTRY=${KIND_LOCAL_REGISTRY:-false} + KIND_DNS_DOMAIN=${KIND_DNS_DOMAIN:-"cluster.local"} KIND_CONFIG=${KIND_CONFIG:-${DIR}/kind.yaml.j2} KIND_REMOVE_TAINT=${KIND_REMOVE_TAINT:-true} KIND_IPV4_SUPPORT=${KIND_IPV4_SUPPORT:-true} @@ -339,26 +389,6 @@ set_default_params() { OCI_BIN=${KIND_EXPERIMENTAL_PROVIDER:-docker} } -detect_apiserver_ip() { - # Detect API_IP used for external communication - # - # You can't use an IPv6 address for the external API, docker does not support - # IPv6 port mapping. Always use the IPv4 host address for the API Server field. - # This will keep compatibility and people will be able to connect with kubectl - # from outside - # - # ip -4 addr -> Run ip command for IPv4 - # grep -oP '(?<=inet\s)\d+(\.\d+){3}' -> Use only the lines with the - # IPv4 Addresses and strip off the trailing subnet mask, /xx - # grep -v "127.0.0.1" -> Remove local host - # head -n 1 -> Of the remaining, use first entry - API_IP=$(ip -4 addr | grep -oP '(?<=inet\s)\d+(\.\d+){3}' | grep -v "127.0.0.1" | head -n 1) - if [ -z "$API_IP" ]; then - echo "Error detecting machine IPv4 to use as API server. Default to 0.0.0.0." - API_IP=0.0.0.0 - fi -} - detect_apiserver_url() { # Detect API_URL used for in-cluster communication # @@ -411,17 +441,17 @@ set_cluster_cidr_ip_families() { IP_FAMILY="" NET_CIDR=$NET_CIDR_IPV4 SVC_CIDR=$SVC_CIDR_IPV4 - echo "IPv4 Only Support: API_IP=$API_IP --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" + echo "IPv4 Only Support: --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" elif [ "$KIND_IPV4_SUPPORT" == false ] && [ "$KIND_IPV6_SUPPORT" == true ]; then IP_FAMILY="ipv6" NET_CIDR=$NET_CIDR_IPV6 SVC_CIDR=$SVC_CIDR_IPV6 - echo "IPv6 Only Support: API_IP=$API_IP --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" + echo "IPv6 Only Support: --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" elif [ "$KIND_IPV4_SUPPORT" == true ] && [ "$KIND_IPV6_SUPPORT" == true ]; then IP_FAMILY="dual" NET_CIDR=$NET_CIDR_IPV4,$NET_CIDR_IPV6 SVC_CIDR=$SVC_CIDR_IPV4,$SVC_CIDR_IPV6 - echo "Dual Stack Support: API_IP=$API_IP --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" + echo "Dual Stack Support: --net-cidr=$NET_CIDR --svc-cidr=$SVC_CIDR" else echo "Invalid setup. KIND_IPV4_SUPPORT and/or KIND_IPV6_SUPPORT must be true." exit 1 @@ -430,13 +460,14 @@ set_cluster_cidr_ip_families() { create_kind_cluster() { # Output of the j2 command - KIND_CONFIG_LCL=${DIR}/kind.yaml + KIND_CONFIG_LCL=${DIR}/kind-${KIND_CLUSTER_NAME}.yaml - ovn_apiServerAddress=${API_IP} \ ovn_ip_family=${IP_FAMILY} \ ovn_ha=${OVN_HA} \ net_cidr=${NET_CIDR} \ svc_cidr=${SVC_CIDR} \ + use_local_registy=${KIND_LOCAL_REGISTRY} \ + dns_domain=${KIND_DNS_DOMAIN} \ ovn_num_master=${KIND_NUM_MASTER} \ ovn_num_worker=${KIND_NUM_WORKER} \ cluster_log_level=${KIND_CLUSTER_LOGLEVEL:-4} \ @@ -499,6 +530,13 @@ coredns_patch() { build_ovn_image() { if [ "$OVN_IMAGE" == local ]; then + # if we're using the local registry and still need to build, push to local registry + if [ "$KIND_LOCAL_REGISTRY" == true ];then + OVN_IMAGE="localhost:5000/ovn-daemonset-f:latest" + else + OVN_IMAGE="localhost/ovn-daemonset-f:dev" + fi + # Build ovn docker image pushd ${DIR}/../go-controller make @@ -509,8 +547,13 @@ build_ovn_image() { # Find all built executables, but ignore the 'windows' directory if it exists find ../../go-controller/_output/go/bin/ -maxdepth 1 -type f -exec cp -f {} . \; echo "ref: $(git rev-parse --symbolic-full-name HEAD) commit: $(git rev-parse HEAD)" > git_info - $OCI_BIN build -t localhost/ovn-daemonset-f:dev -f Dockerfile.fedora . - OVN_IMAGE=localhost/ovn-daemonset-f:dev + $OCI_BIN build -t "${OVN_IMAGE}" -f Dockerfile.fedora . + + # store in local registry + if [ "$KIND_LOCAL_REGISTRY" == true ];then + echo "Pushing built image to local docker registry" + docker push "${OVN_IMAGE}" + fi popd fi } @@ -518,6 +561,7 @@ build_ovn_image() { create_ovn_kube_manifests() { pushd ${DIR}/../dist/images ./daemonset.sh \ + --output-directory="${MANIFEST_OUTPUT_DIR}"\ --image="${OVN_IMAGE}" \ --net-cidr="${NET_CIDR}" \ --svc-cidr="${SVC_CIDR}" \ @@ -547,18 +591,24 @@ create_ovn_kube_manifests() { } install_ovn_image() { - if [ "$OCI_BIN" == "podman" ]; then - # podman: cf https://github.com/kubernetes-sigs/kind/issues/2027 - rm -f /tmp/ovn-kube-f.tar - podman save -o /tmp/ovn-kube-f.tar "${OVN_IMAGE}" - kind load image-archive /tmp/ovn-kube-f.tar --name "${KIND_CLUSTER_NAME}" + # If local registry is being used push image there for consumption by kind cluster + if [ "$KIND_LOCAL_REGISTRY" == true ]; then + echo "OVN-K Image: ${OVN_IMAGE} should already be avaliable in local registry, not loading" else - kind load docker-image "${OVN_IMAGE}" --name "${KIND_CLUSTER_NAME}" - fi + if [ "$OCI_BIN" == "podman" ]; then + # podman: cf https://github.com/kubernetes-sigs/kind/issues/2027 + rm -f /tmp/ovn-kube-f.tar + podman save -o /tmp/ovn-kube-f.tar "${OVN_IMAGE}" + kind load image-archive /tmp/ovn-kube-f.tar --name "${KIND_CLUSTER_NAME}" + else + kind load docker-image "${OVN_IMAGE}" --name "${KIND_CLUSTER_NAME}" + fi + fi } install_ovn() { - pushd ${DIR}/../dist/yaml + pushd ${MANIFEST_OUTPUT_DIR} + run_kubectl apply -f k8s.ovn.org_egressfirewalls.yaml run_kubectl apply -f k8s.ovn.org_egressips.yaml run_kubectl apply -f ovn-setup.yaml @@ -602,7 +652,7 @@ kubectl_wait_pods() { sleep 1 done - if [[ "${PODS_CREATED}" == false ]]; then + if [[ "$PODS_CREATED" == false ]]; then echo "ovn-kubernetes pods were not created." exit 1 fi @@ -643,20 +693,44 @@ sleep_until_pods_settle() { sleep 30 } +# run_script_in_container should be used when kind.sh is run nested in a container +# and makes sure the control-plane node is rechable by substituting 127.0.0.1 +# with the control-plane container's IP +run_script_in_container() { + local master_ip=$(docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' ${KIND_CLUSTER_NAME}-control-plane | head -n 1) + sed -i -- "s/server: .*/server: https:\/\/$master_ip:6443/g" $KUBECONFIG + chmod a+r $KUBECONFIG +} + +# fixup_config_names should be used to ensure kind clusters are named based off +# provided values, essentially it removes the 'kind' prefix from the cluster names +fixup_kubeconfig_names() { + sed -i -- "s/user: kind-.*/user: ${KIND_CLUSTER_NAME}/g" $KUBECONFIG + sed -i -- "s/name: kind-.*/name: ${KIND_CLUSTER_NAME}/g" $KUBECONFIG + sed -i -- "s/cluster: kind-.*/cluster: ${KIND_CLUSTER_NAME}/g" $KUBECONFIG + sed -i -- "s/current-context: .*/current-context: ${KIND_CLUSTER_NAME}/g" $KUBECONFIG +} + check_dependencies -install_j2_renderer # In order to allow providing arguments with spaces, e.g. "-vconsole:info -vfile:info" # the original command was replaced by parse_args "$@" set_default_params print_params + set -euxo pipefail -detect_apiserver_ip check_ipv6 set_cluster_cidr_ip_families create_kind_cluster +if [ "$RUN_IN_CONTAINER" == true ]; then + run_script_in_container +fi +# if cluster name is specified fixup kubeconfig +if [ "$KIND_CLUSTER_NAME"} != "ovn" ]; then + fixup_kubeconfig_names +fi docker_disable_ipv6 -if [ $OVN_ENABLE_EX_GW_NETWORK_BRIDGE == true ]; then +if [ "$OVN_ENABLE_EX_GW_NETWORK_BRIDGE" == true ]; then docker_create_second_interface fi coredns_patch diff --git a/contrib/kind.yaml.j2 b/contrib/kind.yaml.j2 index ed0b898919..41853f3dab 100644 --- a/contrib/kind.yaml.j2 +++ b/contrib/kind.yaml.j2 @@ -5,8 +5,6 @@ networking: kubeProxyMode: "none" # the default CNI will not be installed disableDefaultCNI: true - apiServerAddress: {{ ovn_apiServerAddress | default('0.0.0.0') }} - apiServerPort: 11337 {%- if net_cidr %} podSubnet: "{{ net_cidr }}" {%- endif %} @@ -16,6 +14,16 @@ networking: {%- if ovn_ip_family %} ipFamily: {{ ovn_ip_family }} {%- endif %} +featureGates: +{%- if ovn_ip_family == "dual" %} + IPv6DualStack: true +{%- endif %} +{%- if use_local_registy == "true"%} +containerdConfigPatches: + - |- + [plugins."io.containerd.grpc.v1.cri".registry.mirrors."localhost:5000"] + endpoint = ["http://kind-registry:5000"] +{%- endif %} kubeadmConfigPatches: - | kind: ClusterConfiguration @@ -30,6 +38,8 @@ kubeadmConfigPatches: scheduler: extraArgs: "v": "{{ cluster_log_level }}" + networking: + dnsDomain: {{ dns_domain }} --- kind: InitConfiguration nodeRegistration: diff --git a/dist/images/daemonset.sh b/dist/images/daemonset.sh index 8b4db22b45..8fb8ff2ac3 100755 --- a/dist/images/daemonset.sh +++ b/dist/images/daemonset.sh @@ -7,9 +7,16 @@ set -e # The script renders j2 templates into yaml files in ../yaml/ # ensure j2 renderer installed -pip freeze | grep j2cli || pip install j2cli[yaml] --user -export PATH=~/.local/bin:$PATH +if ! command -v j2 >/dev/null 2>&1 ; then + if ! command -v pip >/dev/null 2>&1 ; then + echo "Dependency not met: 'j2' not installed and cannot install with 'pip'" + exit 1 + fi + echo "'j2' not found, installing with 'pip'" + install_j2_renderer +fi +OVN_OUTPUT_DIR="" OVN_IMAGE="" OVN_IMAGE_PULL_POLICY="" OVN_NET_CIDR="" @@ -64,6 +71,9 @@ while [ "$1" != "" ]; do PARAM=$(echo $1 | awk -F= '{print $1}') VALUE=$(echo $1 | cut -d= -f2-) case $PARAM in + --output-directory) + OVN_OUTPUT_DIR=$VALUE + ;; --image) OVN_IMAGE=$VALUE ;; @@ -232,7 +242,17 @@ while [ "$1" != "" ]; do done # Create the daemonsets with the desired image -# They are expanded into daemonsets in ../yaml +# They are expanded into daemonsets in the specified +# output directory. +if [ -z ${OVN_OUTPUT_DIR} ] ; then + output_dir="../yaml" +else + output_dir=${OVN_OUTPUT_DIR} + if [ ! -d ${OVN_OUTPUT_DIR} ]; then + mkdir $output_dir + fi +fi +echo "output_dir: $output_dir" image=${OVN_IMAGE:-"docker.io/ovnkube/ovn-daemonset:latest"} echo "image: ${image}" @@ -374,7 +394,7 @@ ovn_image=${image} \ ovn_ex_gw_networking_interface=${ovn_ex_gw_networking_interface} \ ovn_disable_ovn_iface_id_ver=${ovn_disable_ovn_iface_id_ver} \ ovnkube_app_name=ovnkube-node \ - j2 ../templates/ovnkube-node.yaml.j2 -o ../yaml/ovnkube-node.yaml + j2 ../templates/ovnkube-node.yaml.j2 -o ${output_dir}/ovnkube-node.yaml # ovnkube node for dpu-host daemonset # TODO: we probably dont need all of these when running on dpu host @@ -406,7 +426,7 @@ ovn_image=${image} \ ovn_ex_gw_networking_interface=${ovn_ex_gw_networking_interface} \ ovnkube_node_mgmt_port_netdev=${ovnkube_node_mgmt_port_netdev} \ ovnkube_app_name=ovnkube-node-dpu-host \ - j2 ../templates/ovnkube-node.yaml.j2 -o ../yaml/ovnkube-node-dpu-host.yaml + j2 ../templates/ovnkube-node.yaml.j2 -o ${output_dir}/ovnkube-node-dpu-host.yaml ovn_image=${image} \ ovn_image_pull_policy=${image_pull_policy} \ @@ -431,7 +451,7 @@ ovn_image=${image} \ ovn_master_count=${ovn_master_count} \ ovn_gateway_mode=${ovn_gateway_mode} \ ovn_ex_gw_networking_interface=${ovn_ex_gw_networking_interface} \ - j2 ../templates/ovnkube-master.yaml.j2 -o ../yaml/ovnkube-master.yaml + j2 ../templates/ovnkube-master.yaml.j2 -o ${output_dir}/ovnkube-master.yaml ovn_image=${image} \ ovn_image_pull_policy=${image_pull_policy} \ @@ -440,7 +460,7 @@ ovn_image=${image} \ ovn_ssl_en=${ovn_ssl_en} \ ovn_nb_port=${ovn_nb_port} \ ovn_sb_port=${ovn_sb_port} \ - j2 ../templates/ovnkube-db.yaml.j2 -o ../yaml/ovnkube-db.yaml + j2 ../templates/ovnkube-db.yaml.j2 -o ${output_dir}/ovnkube-db.yaml ovn_image=${image} \ ovn_image_pull_policy=${image_pull_policy} \ @@ -458,12 +478,12 @@ ovn_image=${image} \ ovn_sb_port=${ovn_sb_port} \ ovn_nb_raft_port=${ovn_nb_raft_port} \ ovn_sb_raft_port=${ovn_sb_raft_port} \ - j2 ../templates/ovnkube-db-raft.yaml.j2 -o ../yaml/ovnkube-db-raft.yaml + j2 ../templates/ovnkube-db-raft.yaml.j2 -o ${output_dir}/ovnkube-db-raft.yaml ovn_image=${image} \ ovn_image_pull_policy=${image_pull_policy} \ ovn_unprivileged_mode=${ovn_unprivileged_mode} \ - j2 ../templates/ovs-node.yaml.j2 -o ../yaml/ovs-node.yaml + j2 ../templates/ovs-node.yaml.j2 -o ${output_dir}/ovs-node.yaml # ovn-setup.yaml net_cidr=${OVN_NET_CIDR:-"10.128.0.0/14/23"} @@ -480,10 +500,10 @@ echo "host_network_namespace: ${host_network_namespace}" net_cidr=${net_cidr} svc_cidr=${svc_cidr} \ mtu_value=${mtu} k8s_apiserver=${k8s_apiserver} \ host_network_namespace=${host_network_namespace} \ - j2 ../templates/ovn-setup.yaml.j2 -o ../yaml/ovn-setup.yaml + j2 ../templates/ovn-setup.yaml.j2 -o ${output_dir}/ovn-setup.yaml -cp ../templates/ovnkube-monitor.yaml.j2 ../yaml/ovnkube-monitor.yaml -cp ../templates/k8s.ovn.org_egressfirewalls.yaml.j2 ../yaml/k8s.ovn.org_egressfirewalls.yaml -cp ../templates/k8s.ovn.org_egressips.yaml.j2 ../yaml/k8s.ovn.org_egressips.yaml +cp ../templates/ovnkube-monitor.yaml.j2 ${output_dir}/ovnkube-monitor.yaml +cp ../templates/k8s.ovn.org_egressfirewalls.yaml.j2 ${output_dir}/k8s.ovn.org_egressfirewalls.yaml +cp ../templates/k8s.ovn.org_egressips.yaml.j2 ${output_dir}/k8s.ovn.org_egressips.yaml exit 0 From 79324257f489be4adce02185c2794c5162628d39 Mon Sep 17 00:00:00 2001 From: astoycos Date: Thu, 10 Feb 2022 14:38:00 -0500 Subject: [PATCH 10/25] Remove ovn-k centos based image builds Centos is now EOL so it no longer makes sense to build ovn-k images for it. This commits deletes the centos based dockerfile and removes the associated make images step. Signed-off-by: astoycos --- dist/images/Dockerfile | 85 ------------------------------------------ dist/images/Makefile | 9 +---- 2 files changed, 1 insertion(+), 93 deletions(-) delete mode 100644 dist/images/Dockerfile diff --git a/dist/images/Dockerfile b/dist/images/Dockerfile deleted file mode 100644 index aa7ebfa3ba..0000000000 --- a/dist/images/Dockerfile +++ /dev/null @@ -1,85 +0,0 @@ -# -# This is the OpenShift ovn overlay network image. -# it provides an overlay network using ovs/ovn/ovn-kube -# -# The standard name for this image is ovn-kube - -# Notes: -# This is for a development build where the ovn-kubernetes utilities -# are built in this Dockerfile and included in the image (instead of the rpm) -# -# This is based on centos:7 -# openvswitch rpms are from -# http://cbs.centos.org/kojifiles/packages/openvswitch/2.9.0/4.el7/x86_64/ -# -# So this file will change over time. - -FROM centos:7 - -USER root - -ENV PYTHONDONTWRITEBYTECODE yes - -COPY kubernetes.repo /etc/yum.repos.d/kubernetes.repo -RUN INSTALL_PKGS=" \ - PyYAML bind-utils procps-ng openssl numactl-libs firewalld-filesystem \ - libpcap kubectl \ - iproute iputils strace socat \ - unbound-libs \ - " && \ - yum install -y --setopt=tsflags=nodocs --setopt=skip_missing_names_on_install=False $INSTALL_PKGS - -RUN yum -y update && yum clean all && rm -rf /var/cache/yum/* - -# Get a reasonable version of openvswitch (2.9.2 or higher) -# docker build --build-arg rpmArch=ARCH -f Dockerfile.centos -t some_tag . -# where ARCH can be x86_64 (default), aarch64, or ppc64le -ARG rpmArch=x86_64 -ARG ovsVer=2.11.0 -ARG ovsSubVer=4.el7 -ARG dpdkVer=18.11.2 -ARG dpdkSubVer=1.el7 -ARG dpdkRpmUrl=http://mirror.centos.org/centos/7/extras/${rpmArch}/Packages/dpdk-${dpdkVer}-${dpdkSubVer}.${rpmArch}.rpm - -RUN if [ "$(uname -m)" = "aarch64" ]; then dpdkRpmUrl=http://mirror.centos.org/altarch/7/extras/aarch64/Packages/dpdk-${dpdkVer}-${dpdkSubVer}.${rpmArch}.rpm; fi && rpm -i ${dpdkRpmUrl} -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-ovn-common-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-ovn-central-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-ovn-host-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-ovn-vtep-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rpm -i http://cbs.centos.org/kojifiles/packages/openvswitch/${ovsVer}/${ovsSubVer}/${rpmArch}/openvswitch-devel-${ovsVer}-${ovsSubVer}.${rpmArch}.rpm -RUN rm -rf /var/cache/yum - -RUN mkdir -p /var/run/openvswitch - -# Built in ../../go_controller, then the binaries are copied here. -# put things where they are in the rpm -RUN mkdir -p /usr/libexec/cni/ -COPY ovnkube ovn-kube-util ovndbchecker /usr/bin/ -COPY ovn-k8s-cni-overlay /usr/libexec/cni/ovn-k8s-cni-overlay - -# ovnkube.sh is the entry point. This script examines environment -# variables to direct operation and configure ovn -COPY ovnkube.sh /root/ -COPY ovndb-raft-functions.sh /root/ -# override the rpm's ovn_k8s.conf with this local copy -COPY ovn_k8s.conf /etc/openvswitch/ovn_k8s.conf - -# copy git commit number into image -COPY git_info /root - -# iptables wrappers -COPY ./iptables-scripts/iptables /usr/sbin/ -COPY ./iptables-scripts/iptables-save /usr/sbin/ -COPY ./iptables-scripts/iptables-restore /usr/sbin/ -COPY ./iptables-scripts/ip6tables /usr/sbin/ -COPY ./iptables-scripts/ip6tables-save /usr/sbin/ -COPY ./iptables-scripts/ip6tables-restore /usr/sbin/ - -LABEL io.k8s.display-name="ovn kubernetes" \ - io.k8s.description="This is a component of OpenShift Container Platform that provides an overlay network using ovn." \ - io.openshift.tags="openshift" \ - maintainer="Tim Rozet " - -WORKDIR /root -ENTRYPOINT /root/ovnkube.sh diff --git a/dist/images/Makefile b/dist/images/Makefile index 3e6cc616b8..096a20cccc 100644 --- a/dist/images/Makefile +++ b/dist/images/Makefile @@ -8,7 +8,7 @@ # The registry is configured in /etc/containers/registries.conf # on each node in both "registries:" and "insecure_registries:" sections. -all: ubuntu centos fedora +all: ubuntu fedora SLASH = - ARCH = $(subst aarch64,arm64,$(subst x86_64,amd64,$(patsubst i%86,386,$(shell uname -m)))) @@ -33,13 +33,6 @@ endif # ${OCI_BIN} push docker.io/ovnkube/ovn-daemonset-u:latest ./daemonset.sh --image=docker.io/ovnkube/ovn-daemonset-u:latest -centos: bld - ${OCI_BIN} build --build-arg rpmArch=$(shell uname -m) -t ovn-daemonset . - ${OCI_BIN} tag ovn-daemonset docker.io/ovnkube/ovn-daemonset:latest - # ${OCI_BIN} login -u ovnkube docker.io/ovnkube - # ${OCI_BIN} push docker.io/ovnkube/ovn-daemonset:latest - ./daemonset.sh --image=docker.io/ovnkube/ovn-daemonset:latest - fedora: bld ${OCI_BIN} build -t ovn-kube-f -f Dockerfile.fedora . # ${OCI_BIN} login -u ovnkube docker.io/ovnkube From 51822056c619a3538304bb78cfe9182d30826c7f Mon Sep 17 00:00:00 2001 From: astoycos Date: Wed, 9 Feb 2022 14:07:54 -0500 Subject: [PATCH 11/25] Remove Dockerfile.ubuntu.arm64 Remove this dockerfile because it is not needed to build ubuntu based arm images, it is a direct copy of Dockerfile.ubuntu, and it is not used within the codebase anywhere. Signed-off-by: astoycos --- dist/images/Dockerfile.ubuntu.arm64 | 53 ----------------------------- 1 file changed, 53 deletions(-) delete mode 100644 dist/images/Dockerfile.ubuntu.arm64 diff --git a/dist/images/Dockerfile.ubuntu.arm64 b/dist/images/Dockerfile.ubuntu.arm64 deleted file mode 100644 index affaf03dff..0000000000 --- a/dist/images/Dockerfile.ubuntu.arm64 +++ /dev/null @@ -1,53 +0,0 @@ -# -# The standard name for this image is ovn-kube-u - -# Notes: -# This is for a development build where the ovn-kubernetes utilities -# are built in this Dockerfile and included in the image (instead of the deb package) -# -# -# So this file will change over time. - -FROM ubuntu:19.10 - -USER root - -RUN apt-get update && apt-get install -y iproute2 curl software-properties-common util-linux - -RUN echo "deb https://apt.kubernetes.io/ kubernetes-xenial main" | tee -a /etc/apt/sources.list.d/kubernetes.list -RUN curl -s https://packages.cloud.google.com/apt/doc/apt-key.gpg | apt-key add - - -# Install OVS and OVN packages. -RUN apt-get update && apt-get install -y openvswitch-switch openvswitch-common ovn-central ovn-common ovn-host kubectl - -RUN mkdir -p /var/run/openvswitch - -# Built in ../../go_controller, then the binaries are copied here. -# put things where they are in the pkg -RUN mkdir -p /usr/libexec/cni/ -COPY ovnkube ovn-kube-util ovndbchecker /usr/bin/ -COPY ovn-k8s-cni-overlay /usr/libexec/cni/ovn-k8s-cni-overlay - -# ovnkube.sh is the entry point. This script examines environment -# variables to direct operation and configure ovn -COPY ovnkube.sh /root/ -COPY ovndb-raft-functions.sh /root/ -# override the pkg's ovn_k8s.conf with this local copy -COPY ovn_k8s.conf /etc/openvswitch/ovn_k8s.conf - -# copy git commit number into image -COPY git_info /root - -# iptables wrappers -COPY ./iptables-scripts/iptables /usr/sbin/ -COPY ./iptables-scripts/iptables-save /usr/sbin/ -COPY ./iptables-scripts/iptables-restore /usr/sbin/ -COPY ./iptables-scripts/ip6tables /usr/sbin/ -COPY ./iptables-scripts/ip6tables-save /usr/sbin/ -COPY ./iptables-scripts/ip6tables-restore /usr/sbin/ - -LABEL io.k8s.display-name="ovn kubernetes" \ - io.k8s.description="ovnkube ubuntu image" - -WORKDIR /root -ENTRYPOINT /root/ovnkube.sh From c261990b6d1590e1a3dc0aa69ba28d5a64c60bcb Mon Sep 17 00:00:00 2001 From: astoycos Date: Tue, 1 Mar 2022 16:11:35 -0500 Subject: [PATCH 12/25] Push ovn-k images to ghcr.io Automatically push the built ovn-k images with fedora, and ubuntu bases(arm64 amd6d) to ghcr.io for the ovn-org. New images will be built and push every time new changes are merged into the ovn-kubernetes master branch. Update the ovn-docker-images workflow to only run when code is merged into master. Signed-off-by: astoycos --- .github/workflows/docker.yml | 77 ++++++++++++++++++++++++++++++++--- README.md | 8 ++++ dist/images/Dockerfile.fedora | 9 +++- 3 files changed, 88 insertions(+), 6 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index de3097a272..90f9b7a930 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -2,12 +2,15 @@ name: ovn-docker-images on: push: - pull_request: branches: [ master ] - paths: [ "dist/images/Dockerfile*"] env: GO_VERSION: 1.17.6 + REGISTRY: ghcr.io + OWNER: ovn-org + REPOSITORY: ovn-kubernetes + FEDORA_IMAGE_NAME: ovn-kube-f + UBUNTU_IMAGE_NAME: ovn-kube-u jobs: build: @@ -23,12 +26,76 @@ jobs: - name: Check out code into the Go module directory uses: actions/checkout@v2 + - name: Log in to the GH Container registry + uses: docker/login-action@v1.12.0 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + - name: Set up environment run: | export GOPATH=$(go env GOPATH) echo "GOPATH=$GOPATH" >> $GITHUB_ENV echo "$GOPATH/bin" >> $GITHUB_PATH - - name: Build images - run: | - make -C dist/images all + - name: Build ovnkube-binaries copy to context + run: | + pushd go-controller + make + popd + + pushd dist/images + cp -r ../../go-controller/_output/go/bin/* . + popd + + - name: Generate git-info to write to image + run: | + BRANCH=$(git rev-parse --short "$GITHUB_SHA") + COMMIT=$(git rev-parse HEAD) + pushd dist/images + echo "ref: ${BRANCH} commit: ${COMMIT}" > git_info + popd + + - name: Set up QEMU + uses: docker/setup-qemu-action@master + with: + platforms: all + + - name: Set up Docker Buildx + id: buildx + uses: docker/setup-buildx-action@master + + - name: Extract metadata (tags, labels) for fedora ovn-k image + id: meta-fedora + uses: docker/metadata-action@v3.6.2 + with: + images: ${{ env.REGISTRY }}/${{ env.OWNER }}/${{ env.REPOSITORY }}/${{ env.FEDORA_IMAGE_NAME }} + + - name: Build and push Fedora based Docker image + uses: docker/build-push-action@v2.9.0 + with: + builder: ${{ steps.buildx.outputs.name }} + context: ./dist/images + file: ./dist/images/Dockerfile.fedora + push: true + platforms: linux/amd64,linux/arm64 + tags: ${{ steps.meta-fedora.outputs.tags }} + labels: ${{ steps.meta-fedora.outputs.labels }} + + - name: Extract metadata (tags, labels) for ubuntu ovn-k image + id: meta-ubuntu + uses: docker/metadata-action@v3.6.2 + with: + images: ${{ env.REGISTRY }}/${{ env.OWNER }}/${{ env.REPOSITORY }}/${{ env.UBUNTU_IMAGE_NAME }} + + - name: Build and push Ubuntu based Docker image + uses: docker/build-push-action@v2.9.0 + with: + builder: ${{ steps.buildx.outputs.name }} + context: ./dist/images + file: ./dist/images/Dockerfile.ubuntu + push: true + platforms: linux/amd64,linux/arm64 + tags: ${{ steps.meta-ubuntu.outputs.tags }} + labels: ${{ steps.meta-ubuntu.outputs.labels }} diff --git a/README.md b/README.md index c2390fdd74..3c1b49c765 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,14 @@ On Linux, the easiest way to get started is to use OVN DaemonSet and Deployments. +# Master Based Ovn-Kubernetes Images + +With every PR that is merged into master, ovn-kubernetes images are automatically +rebuilt and pushed to ghcr.io (i.e [ovn-org's packages](https://github.com/orgs/ovn-org/packages)) +for consumption. They are built with fedora, and ubuntu base images both of which +are built for arm64 and amd64 architectures. These are not official releases and are just provided to make +using ovn-kubernetes easier for other projects. + ## Install Open vSwitch kernel modules on all hosts. Most Linux distributions come with Open vSwitch kernel module by default. You diff --git a/dist/images/Dockerfile.fedora b/dist/images/Dockerfile.fedora index 486079cbc0..90282ee347 100644 --- a/dist/images/Dockerfile.fedora +++ b/dist/images/Dockerfile.fedora @@ -16,6 +16,11 @@ USER root ENV PYTHONDONTWRITEBYTECODE yes ARG ovnver=ovn-21.12.0-3.fc35 +# Automatically populated when using docker buildx +ARG TARGETPLATFORM +ARG BUILDPLATFORM + +RUN echo "Running on $BUILDPLATFORM, building for $TARGETPLATFORM" # install needed rpms - openvswitch must be 2.10.4 or higher RUN INSTALL_PKGS=" \ @@ -29,7 +34,9 @@ RUN INSTALL_PKGS=" \ RUN mkdir -p /var/run/openvswitch -RUN koji download-build $ovnver --arch=x86_64 +RUN if [ "$TARGETPLATFORM" = "linux/amd64" ] || [ -z "$TARGETPLATFORM"] ; then koji download-build $ovnver --arch=x86_64 ; \ + else koji download-build $ovnver --arch=aarch64 ; fi + RUN rpm -Uhv --nodeps --force *.rpm # Built in ../../go_controller, then the binaries are copied here. From ef2b7f8ddd391dc5555cd0c7db9b8d05a3e43e68 Mon Sep 17 00:00:00 2001 From: astoycos Date: Wed, 12 Jan 2022 09:40:49 -0500 Subject: [PATCH 13/25] Update default KUBECONFIG file Update the location of the KUBECONFIG file from admin.conf -> ovn.conf Signed-off-by: astoycos --- contrib/kind-dual-stack-conversion.sh | 2 +- docs/INSTALL.KUBEADM.md | 2 +- docs/ci.md | 6 +++--- docs/kind.md | 12 ++++++------ test/scripts/e2e-cp.sh | 2 +- test/scripts/e2e-kind.sh | 2 +- test/scripts/upgrade-ovn.sh | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/contrib/kind-dual-stack-conversion.sh b/contrib/kind-dual-stack-conversion.sh index b7c7bc0e43..01d7890776 100755 --- a/contrib/kind-dual-stack-conversion.sh +++ b/contrib/kind-dual-stack-conversion.sh @@ -103,7 +103,7 @@ SECONDARY_SERVICE_SUBNET=${SECONDARY_SERVICE_SUBNET:-"fd00:10:96::/112"} SECONDARY_CLUSTER_SUBNET=${SECONDARY_CLUSTER_SUBNET:-"fd00:10:244::/56"} # NOTE: ovn only -export KUBECONFIG=${HOME}/admin.conf +export KUBECONFIG=${HOME}/ovn.conf # KIND nodes NODES=$(kind get nodes --name ${CLUSTER_NAME}) diff --git a/docs/INSTALL.KUBEADM.md b/docs/INSTALL.KUBEADM.md index 9f5896c318..d3eea5f64b 100644 --- a/docs/INSTALL.KUBEADM.md +++ b/docs/INSTALL.KUBEADM.md @@ -387,7 +387,7 @@ Deploy on the master node `node1`: ~~~ kubeadm init --pod-network-cidr 172.16.0.0/16 --service-cidr 172.17.0.0/16 --apiserver-advertise-address 192.168.123.1 mkdir -p $HOME/.kube -sudo cp -i /etc/kubernetes/admin.conf $HOME/.kube/config +sudo cp -i /etc/kubernetes/ovn.conf $HOME/.kube/config sudo chown $(id -u):$(id -g) $HOME/.kube/config ~~~ diff --git a/docs/ci.md b/docs/ci.md index 9009851c10..4e60dfc463 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -192,7 +192,7 @@ To run the tests locally, run a KIND deployment as described above. The E2E tests look for the kube config file in a special location, so make a copy: ``` -cp ~/admin.conf ~/.kube/kind-config-kind +cp ~/ovn.conf ~/.kube/kind-config-kind ``` To run the desired shard, first make sure that the necessary environment variables are exported (see section above). Then, go to the location of your local copy of the `ovn-kubernetes` repository: @@ -255,7 +255,7 @@ For example: + NUM_NODES=20 + export NUM_WORKER_NODES=3 + NUM_WORKER_NODES=3 -+ ginkgo --nodes=20 '--focus=should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' '--skip=Networking\sIPerf\sIPv[46]|\[Feature:PerformanceDNS\]|Disruptive|DisruptionController|\[sig-apps\]\sCronJob|\[sig-storage\]|\[Feature:Federation\]|should\shave\sipv4\sand\sipv6\sinternal\snode\sip|should\shave\sipv4\sand\sipv6\snode\spodCIDRs|kube-proxy|should\sset\sTCP\sCLOSE_WAIT\stimeout|should\shave\ssession\saffinity\stimeout\swork|named\sport.+\[Feature:NetworkPolicy\]|\[Feature:SCTP\]|service.kubernetes.io/headless|should\sresolve\sconnection\sreset\sissue\s#74839|sig-api-machinery|\[Feature:NoSNAT\]|Services.+(ESIPP|cleanup\sfinalizer)|configMap\snameserver|ClusterDns\s\[Feature:Example\]|should\sset\sdefault\svalue\son\snew\sIngressClass|should\sprevent\sIngress\screation\sif\smore\sthan\s1\sIngressClass\smarked\sas\sdefault|\[Feature:Networking-IPv6\]|\[Feature:.*DualStack.*\]' --flakeAttempts=5 /usr/local/bin/e2e.test -- --kubeconfig=/root/admin.conf --provider=local --dump-logs-on-failure=false --report-dir=/root/ovn-kubernetes/test/_artifacts --disable-log-dump=true --num-nodes=3 ++ ginkgo --nodes=20 '--focus=should\sbe\sable\sto\spreserve\sUDP\straffic\swhen\sserver\spod\scycles\sfor\sa\sNodePort\sservice' '--skip=Networking\sIPerf\sIPv[46]|\[Feature:PerformanceDNS\]|Disruptive|DisruptionController|\[sig-apps\]\sCronJob|\[sig-storage\]|\[Feature:Federation\]|should\shave\sipv4\sand\sipv6\sinternal\snode\sip|should\shave\sipv4\sand\sipv6\snode\spodCIDRs|kube-proxy|should\sset\sTCP\sCLOSE_WAIT\stimeout|should\shave\ssession\saffinity\stimeout\swork|named\sport.+\[Feature:NetworkPolicy\]|\[Feature:SCTP\]|service.kubernetes.io/headless|should\sresolve\sconnection\sreset\sissue\s#74839|sig-api-machinery|\[Feature:NoSNAT\]|Services.+(ESIPP|cleanup\sfinalizer)|configMap\snameserver|ClusterDns\s\[Feature:Example\]|should\sset\sdefault\svalue\son\snew\sIngressClass|should\sprevent\sIngress\screation\sif\smore\sthan\s1\sIngressClass\smarked\sas\sdefault|\[Feature:Networking-IPv6\]|\[Feature:.*DualStack.*\]' --flakeAttempts=5 /usr/local/bin/e2e.test -- --kubeconfig=/root/ovn.conf --provider=local --dump-logs-on-failure=false --report-dir=/root/ovn-kubernetes/test/_artifacts --disable-log-dump=true --num-nodes=3 Running Suite: Kubernetes e2e suite (...) • [SLOW TEST:28.091 seconds] @@ -303,7 +303,7 @@ For example: ./e2e/multicast.go: ginkgo.It("should be able to send multicast UDP traffic between nodes", func() { # make control-plane WHAT="should be able to send multicast UDP traffic between nodes" (...) -+ go test -timeout=0 -v . -ginkgo.v -ginkgo.focus 'should\sbe\sable\sto\ssend\smulticast\sUDP\straffic\sbetween\snodes' -ginkgo.flakeAttempts 2 '-ginkgo.skip=recovering from deleting db files while maintain connectivity|Should validate connectivity before and after deleting all the db-pods at once in HA mode|Should be allowed to node local cluster-networked endpoints by nodeport services with externalTrafficPolicy=local|e2e ingress to host-networked pods traffic validation|host to host-networked pods traffic validation' -provider skeleton -kubeconfig /root/admin.conf --num-nodes=2 --report-dir=/root/ovn-kubernetes/test/_artifacts --report-prefix=control-plane_ ++ go test -timeout=0 -v . -ginkgo.v -ginkgo.focus 'should\sbe\sable\sto\ssend\smulticast\sUDP\straffic\sbetween\snodes' -ginkgo.flakeAttempts 2 '-ginkgo.skip=recovering from deleting db files while maintain connectivity|Should validate connectivity before and after deleting all the db-pods at once in HA mode|Should be allowed to node local cluster-networked endpoints by nodeport services with externalTrafficPolicy=local|e2e ingress to host-networked pods traffic validation|host to host-networked pods traffic validation' -provider skeleton -kubeconfig /root/ovn.conf --num-nodes=2 --report-dir=/root/ovn-kubernetes/test/_artifacts --report-prefix=control-plane_ I0817 15:26:21.762483 1197731 test_context.go:457] Tolerating taints "node-role.kubernetes.io/master" when considering if nodes are ready === RUN TestE2e I0817 15:26:21.762635 1197731 e2e_suite_test.go:67] Saving reports to /root/ovn-kubernetes/test/_artifacts diff --git a/docs/kind.md b/docs/kind.md index b6f09d2436..1fc2d40a1f 100644 --- a/docs/kind.md +++ b/docs/kind.md @@ -46,7 +46,7 @@ Launch the KIND Deployment. ``` $ pushd contrib -$ export KUBECONFIG=${HOME}/admin.conf +$ export KUBECONFIG=${HOME}/ovn.conf $ ./kind.sh $ popd ``` @@ -152,7 +152,7 @@ To deploy KIND however, you need to start it as root and then copy root's kube c ``` $ pushd contrib $ sudo ./kind.sh -ep podman -$ sudo cp /root/admin.conf ~/.kube/kind-config +$ sudo cp /root/ovn.conf ~/.kube/kind-config $ sudo chown $(id -u):$(id -g) ~/.kube/kind-config $ export KUBECONFIG=~/.kube/kind-config $ popd @@ -309,9 +309,9 @@ $ KIND_IPV4_SUPPORT=false KIND_IPV6_SUPPORT=true ./kind.sh Once `kind.sh` completes, setup kube config file: ``` -$ cp ~/admin.conf ~/.kube/config +$ cp ~/ovn.conf ~/.kube/config -- OR -- -$ KUBECONFIG=~/admin.conf +$ KUBECONFIG=~/ovn.conf ``` Once testing is complete, to tear down the KIND deployment: @@ -412,9 +412,9 @@ $ KIND_IPV4_SUPPORT=true KIND_IPV6_SUPPORT=true K8S_VERSION=v1.23.3 ./kind.sh Once `kind.sh` completes, setup kube config file: ``` -$ cp ~/admin.conf ~/.kube/config +$ cp ~/ovn.conf ~/.kube/config -- OR -- -$ KUBECONFIG=~/admin.conf +$ KUBECONFIG=~/ovn.conf ``` Once testing is complete, to tear down the KIND deployment: diff --git a/test/scripts/e2e-cp.sh b/test/scripts/e2e-cp.sh index a97303617c..9c67c1bc40 100755 --- a/test/scripts/e2e-cp.sh +++ b/test/scripts/e2e-cp.sh @@ -4,7 +4,7 @@ set -ex # setting this env prevents ginkgo e2e from trying to run provider setup export KUBERNETES_CONFORMANCE_TEST=y -export KUBECONFIG=${HOME}/admin.conf +export KUBECONFIG=${HOME}/ovn.conf # Skip tests which are not IPv6 ready yet (see description of https://github.com/ovn-org/ovn-kubernetes/pull/2276) # (Note that netflow v5 is IPv4 only) diff --git a/test/scripts/e2e-kind.sh b/test/scripts/e2e-kind.sh index 102d8ca04c..43bff6568c 100755 --- a/test/scripts/e2e-kind.sh +++ b/test/scripts/e2e-kind.sh @@ -166,7 +166,7 @@ ginkgo --nodes=${NUM_NODES} \ --flakeAttempts=${FLAKE_ATTEMPTS} \ /usr/local/bin/e2e.test \ -- \ - --kubeconfig=${HOME}/admin.conf \ + --kubeconfig=${HOME}/ovn.conf \ --provider=local \ --dump-logs-on-failure=false \ --report-dir=${E2E_REPORT_DIR} \ diff --git a/test/scripts/upgrade-ovn.sh b/test/scripts/upgrade-ovn.sh index 676703ba7c..a7c40fb56d 100755 --- a/test/scripts/upgrade-ovn.sh +++ b/test/scripts/upgrade-ovn.sh @@ -4,7 +4,7 @@ set -ex -export KUBECONFIG=${HOME}/admin.conf +export KUBECONFIG=${HOME}/ovn.conf export OVN_IMAGE=${OVN_IMAGE:-ovn-daemonset-f:pr} kubectl_wait_pods() { From 5a51d58b56efc275cca8e024207994a4391f9313 Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Tue, 1 Mar 2022 18:32:00 +0000 Subject: [PATCH 14/25] Only start and register control plane recorder for leader We should only register and start the metrics if we are current leader otherwise we are confusing the user with 0 value metrics on non-leaders and wasting CPU cycles. Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/master.go | 28 +++++++++++++++------------- go-controller/pkg/ovn/master.go | 2 ++ go-controller/pkg/ovn/ovn.go | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index f5eb48f280..59075cddca 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -311,7 +311,6 @@ func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclie prometheus.MustRegister(metricIPsecEnabled) prometheus.MustRegister(metricEgressFirewallCount) prometheus.MustRegister(metricEgressRoutingViaHost) - registerControlPlaneRecorderMetrics() registerWorkqueueMetrics(MetricOvnkubeNamespace, MetricOvnkubeSubsystemMaster) // register DBe2eTimestamp to ensure northd is not hung @@ -483,13 +482,6 @@ func DecrementEgressFirewallCount() { metricEgressFirewallCount.Dec() } -func registerControlPlaneRecorderMetrics() { - prometheus.MustRegister(metricFirstSeenLSPLatency) - prometheus.MustRegister(metricLSPPortBindingLatency) - prometheus.MustRegister(metricPortBindingUpLatency) - prometheus.MustRegister(metricPortBindingChassisLatency) -} - type timestampType int const ( @@ -514,19 +506,29 @@ type ControlPlaneRecorder struct { podRecords map[kapimtypes.UID]*record } -func NewControlPlaneRecorder(sbClient libovsdbclient.Client) *ControlPlaneRecorder { - recorder := ControlPlaneRecorder{sync.Mutex{}, make(map[kapimtypes.UID]*record)} +func NewControlPlaneRecorder() *ControlPlaneRecorder { + return &ControlPlaneRecorder{ + podRecords: make(map[kapimtypes.UID]*record), + } +} + +func (ps *ControlPlaneRecorder) Run(sbClient libovsdbclient.Client) { + // only register the metrics when we want them + prometheus.MustRegister(metricFirstSeenLSPLatency) + prometheus.MustRegister(metricLSPPortBindingLatency) + prometheus.MustRegister(metricPortBindingUpLatency) + prometheus.MustRegister(metricPortBindingChassisLatency) + sbClient.Cache().AddEventHandler(&cache.EventHandlerFuncs{ AddFunc: func(table string, model model.Model) { - go recorder.AddPortBindingEvent(table, model) + go ps.AddPortBindingEvent(table, model) }, UpdateFunc: func(table string, old model.Model, new model.Model) { - go recorder.UpdatePortBindingEvent(table, old, new) + go ps.UpdatePortBindingEvent(table, old, new) }, DeleteFunc: func(table string, model model.Model) { }, }) - return &recorder } func (ps *ControlPlaneRecorder) AddPodEvent(podUID kapimtypes.UID) { diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go index d415977354..36ac595432 100644 --- a/go-controller/pkg/ovn/master.go +++ b/go-controller/pkg/ovn/master.go @@ -215,6 +215,8 @@ func (oc *Controller) upgradeOVNTopology(existingNodes *kapi.NodeList) error { func (oc *Controller) StartClusterMaster(masterNodeName string) error { klog.Infof("Starting cluster master") + oc.metricsRecorder.Run(oc.sbClient) + // enableOVNLogicalDataPathGroups sets an OVN flag to enable logical datapath // groups on OVN 20.12 and later. The option is ignored if OVN doesn't // understand it. Logical datapath groups reduce the size of the southbound diff --git a/go-controller/pkg/ovn/ovn.go b/go-controller/pkg/ovn/ovn.go index 1112e79467..3c12881d54 100644 --- a/go-controller/pkg/ovn/ovn.go +++ b/go-controller/pkg/ovn/ovn.go @@ -311,7 +311,7 @@ func NewOvnController(ovnClient *util.OVNClientset, wf *factory.WatchFactory, st svcController: svcController, svcFactory: svcFactory, modelClient: modelClient, - metricsRecorder: metrics.NewControlPlaneRecorder(libovsdbOvnSBClient), + metricsRecorder: metrics.NewControlPlaneRecorder(), } } From 84eb498213e2d2674e45711192db51a73a968c25 Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Wed, 2 Mar 2022 15:04:21 +0000 Subject: [PATCH 15/25] Metrics: register/start timestamp/ipsec if leader Prior to this change, timestamping of NB and IPSec both registered and started their metrics when ovn kube master was not leader. Functionality stays the same except 'e2e_timestamp' metric is registered with the non-ovn registry because this registry is not exposed in ovnkube master. Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/master.go | 224 ++++++++++++++-------------- go-controller/pkg/ovn/master.go | 5 +- 2 files changed, 114 insertions(+), 115 deletions(-) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index 59075cddca..c410610816 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -23,14 +23,27 @@ import ( klog "k8s.io/klog/v2" ) -// metricE2ETimestamp is a timestamp value we have persisted to nbdb. We will -// also export a metric with the same column in sbdb. We will also bump this -// every 30 seconds, so we can detect a hung northd. -var metricE2ETimestamp = prometheus.NewGauge(prometheus.GaugeOpts{ +// metricNbE2eTimestamp is the UNIX timestamp value set to NB DB. A corresponding metric +// 'sb_e2e_timestamp' for SB DB will contain the timestamp that was written to NB DB. +// This is registered within func RunTimestamp in order to allow gathering this metric on +// the fly when metrics are scraped. +var metricNbE2eTimestamp = prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: MetricOvnkubeNamespace, Subsystem: MetricOvnkubeSubsystemMaster, Name: "nb_e2e_timestamp", - Help: "The current e2e-timestamp value as written to the northbound database"}) + Help: "The current e2e-timestamp value as written to the northbound database"}, +) + +// metricDbTimestamp is the UNIX timestamp seen in NB and SB DBs. +var metricDbTimestamp = prometheus.NewGaugeVec(prometheus.GaugeOpts{ + Namespace: MetricOvnNamespace, + Subsystem: MetricOvnSubsystemDB, + Name: "e2e_timestamp", + Help: "The current e2e-timestamp value as observed in this instance of the database"}, + []string{ + "db_name", + }, +) // metricPodCreationLatency is the time between a pod being scheduled and the // ovn controller setting the network annotations. @@ -233,7 +246,6 @@ var metricPortBindingUpLatency = prometheus.NewHistogram(prometheus.HistogramOpt }) var registerMasterMetricsOnce sync.Once -var startMasterMetricUpdaterOnce sync.Once // RegisterMasterMetrics registers some ovnkube master metrics with the Prometheus // registry @@ -242,28 +254,8 @@ func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclie // ovnkube-master metrics // the updater for this metric is activated // after leader election - prometheus.MustRegister(metricE2ETimestamp) prometheus.MustRegister(MetricMasterLeader) prometheus.MustRegister(metricPodCreationLatency) - - scrapeOvnTimestamp := func() float64 { - sbGlobal, err := libovsdbops.FindSBGlobal(sbClient) - if err != nil { - klog.Errorf("Failed to get global options for the SB_Global table err: %v", err) - return 0 - } - if val, ok := sbGlobal.Options["e2e_timestamp"]; ok { - return parseMetricToFloat(MetricOvnkubeSubsystemMaster, "sb_e2e_timestamp", val) - } - return 0 - } - prometheus.MustRegister(prometheus.NewGaugeFunc( - prometheus.GaugeOpts{ - Namespace: MetricOvnkubeNamespace, - Subsystem: MetricOvnkubeSubsystemMaster, - Name: "sb_e2e_timestamp", - Help: "The current e2e-timestamp value as observed in the southbound database", - }, scrapeOvnTimestamp)) prometheus.MustRegister(prometheus.NewCounterFunc( prometheus.CounterOpts{ Namespace: MetricOvnkubeNamespace, @@ -308,21 +300,9 @@ func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclie prometheus.MustRegister(metricV6AllocatedHostSubnetCount) prometheus.MustRegister(metricEgressIPCount) prometheus.MustRegister(metricEgressFirewallRuleCount) - prometheus.MustRegister(metricIPsecEnabled) prometheus.MustRegister(metricEgressFirewallCount) prometheus.MustRegister(metricEgressRoutingViaHost) registerWorkqueueMetrics(MetricOvnkubeNamespace, MetricOvnkubeSubsystemMaster) - - // register DBe2eTimestamp to ensure northd is not hung - ovnRegistry.MustRegister(metricDBE2eTimestamp) - go func() { - for { - ovnE2eTimeStampUpdater("nb", "OVN_Northbound", nbClient, sbClient) - ovnE2eTimeStampUpdater("sb", "OVN_Southbound", nbClient, sbClient) - time.Sleep(30 * time.Second) - } - }() - prometheus.MustRegister(prometheus.NewGaugeFunc( prometheus.GaugeOpts{ Namespace: MetricOvnNamespace, @@ -353,25 +333,57 @@ func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclie }) } -// StartMasterMetricUpdater adds a goroutine that updates a "timestamp" value in the -// nbdb every 30 seconds. This is so we can determine freshness of the database. -// Also, update IPsec enabled or disable metric. -func StartMasterMetricUpdater(stopChan <-chan struct{}, nbClient libovsdbclient.Client) { - startMasterMetricUpdaterOnce.Do(func() { - addIPSecMetricHandler(nbClient) - go func() { - ticker := time.NewTicker(30 * time.Second) - defer ticker.Stop() - for { - select { - case <-ticker.C: - updateE2ETimestampMetric(nbClient) - case <-stopChan: - return +// RunTimestamp adds a goroutine that registers and updates timestamp metrics. +// This is so we can determine 'freshness' of the components NB/SB DB and northd. +// Function must be called once. +func RunTimestamp(stopChan <-chan struct{}, sbClient, nbClient libovsdbclient.Client) { + // Metric named nb_e2e_timestamp is the UNIX timestamp this instance wrote to NB DB. Updated every 30s with the + // current timestamp. + prometheus.MustRegister(metricNbE2eTimestamp) + + // Metric named sb_e2e_timestamp is the UNIX timestamp observed in SB DB. The value is read from the SB DB + // cache when metrics HTTP endpoint is scraped. + scrapeOvnSbE2eTimestamp := func() float64 { + sbGlobal, err := libovsdbops.FindSBGlobal(sbClient) + if err != nil { + klog.Errorf("Failed to get global options for the SB_Global table: %v", err) + return 0 + } + if val, ok := sbGlobal.Options["e2e_timestamp"]; ok { + return parseMetricToFloat(MetricOvnkubeSubsystemMaster, "sb_e2e_timestamp", val) + } + return 0 + } + prometheus.MustRegister(prometheus.NewGaugeFunc( + prometheus.GaugeOpts{ + Namespace: MetricOvnkubeNamespace, + Subsystem: MetricOvnkubeSubsystemMaster, + Name: "sb_e2e_timestamp", + Help: "The current e2e-timestamp value as observed in the southbound database", + }, scrapeOvnSbE2eTimestamp)) + + // Metric named e2e_timestamp is the UNIX timestamp observed in NB and SB DBs cache with the DB name + // (OVN_Northbound|OVN_Southbound) set as a label. Updated every 30s. + prometheus.MustRegister(metricDbTimestamp) + + go func() { + ticker := time.NewTicker(30 * time.Second) + defer ticker.Stop() + for { + select { + case <-ticker.C: + currentTime := time.Now().Unix() + if setNbE2eTimestamp(nbClient, currentTime) { + metricNbE2eTimestamp.Set(float64(currentTime)) } + + metricDbTimestamp.WithLabelValues(nbClient.Schema().Name).Set(getDbOptionsTimestamp(nbClient)) + metricDbTimestamp.WithLabelValues(sbClient.Schema().Name).Set(getDbOptionsTimestamp(sbClient)) + case <-stopChan: + return } - }() - }) + } + }() } // RecordPodCreated extracts the scheduled timestamp and records how long it took @@ -435,18 +447,11 @@ func RecordEgressRoutingViaHost() { } } -func updateE2ETimestampMetric(ovnNBClient libovsdbclient.Client) { - currentTime := time.Now().Unix() - // assumption that only first row is relevant in NB_Global table - if err := libovsdbops.UpdateNBGlobalOptions(ovnNBClient, map[string]string{"e2e_timestamp": fmt.Sprintf("%d", currentTime)}); err != nil { - klog.Errorf("Unable to update E2E timestamp metric err: %v", err) - return - } - - metricE2ETimestamp.Set(float64(currentTime)) -} - -func addIPSecMetricHandler(ovnNBClient libovsdbclient.Client) { +// MonitorIPSec will register a metric to determine if IPSec is enabled/disabled. It will also add a handler +// to NB libovsdb cache to update the IPSec metric. +// This function should only be called once. +func MonitorIPSec(ovnNBClient libovsdbclient.Client) { + prometheus.MustRegister(metricIPsecEnabled) ovnNBClient.Cache().AddEventHandler(&cache.EventHandlerFuncs{ AddFunc: func(table string, model model.Model) { ipsecMetricHandler(table, model) @@ -636,61 +641,54 @@ func getPodUIDFromPortBinding(row *sbdb.PortBinding) kapimtypes.UID { return kapimtypes.UID(podUID) } -var metricDBE2eTimestamp = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: MetricOvnNamespace, - Subsystem: MetricOvnSubsystemDB, - Name: "e2e_timestamp", - Help: "The current e2e-timestamp value as observed in this instance of the database"}, - []string{ - "db_name", - }, -) - -func ovnE2eTimeStampUpdater(direction, database string, nbClient libovsdbclient.Client, - sbClient libovsdbclient.Client) { - var nbGlobal *nbdb.NBGlobal - var sbGlobal *sbdb.SBGlobal - var err error +// setNbE2eTimestamp return true if setting timestamp to NB global options is successful +func setNbE2eTimestamp(ovnNBClient libovsdbclient.Client, timestamp int64) bool { + // assumption that only first row is relevant in NB_Global table + options := map[string]string{"e2e_timestamp": fmt.Sprintf("%d", timestamp)} + if err := libovsdbops.UpdateNBGlobalOptions(ovnNBClient, options); err != nil { + klog.Errorf("Unable to update NB global options E2E timestamp metric err: %v", err) + return false + } + return true +} - scrapeAndSetE2ETimestampMetrics := func(options map[string]string) { - var v string - var ok bool +func getDbOptionsTimestamp(client libovsdbclient.Client) float64 { + var options map[string]string + dbName := client.Schema().Name - if v, ok = options["e2e_timestamp"]; !ok { - klog.Error("Failed to scrape timestamp from global config options") - return + if dbName == "OVN_Northbound" { + if nbGlobal, err := libovsdbops.FindNBGlobal(client); err != nil && err != libovsdbclient.ErrNotFound { + klog.Errorf("Failed to get NB_Global table err: %v", err) + return 0 + } else { + options = nbGlobal.Options } + } - if value, err := strconv.ParseFloat(v, 64); err == nil { - metricDBE2eTimestamp.WithLabelValues(database).Set(value) + if dbName == "OVN_Southbound" { + if sbGlobal, err := libovsdbops.FindSBGlobal(client); err != nil && err != libovsdbclient.ErrNotFound { + klog.Errorf("Failed to get SB_Global table err: %v", err) + return 0 } else { - klog.Errorf("Failed to parse e2e-timestamp value to float64 err: %v", err) + options = sbGlobal.Options } } + return extractOptionsTimestamp(options, dbName) +} - if direction == "sb" { - if sbGlobal, err = libovsdbops.FindSBGlobal(sbClient); err != nil && err != libovsdbclient.ErrNotFound { - klog.Errorf("Failed to scrape timestamp from sb_Global table "+ - "err: %v", err) - return - } +func extractOptionsTimestamp(options map[string]string, name string) float64 { + var v string + var ok bool - if sbGlobal != nil { - scrapeAndSetE2ETimestampMetrics(sbGlobal.Options) - } else { - metricDBE2eTimestamp.WithLabelValues(database).Set(0) - } - } else { - if nbGlobal, err = libovsdbops.FindNBGlobal(nbClient); err != nil && err != libovsdbclient.ErrNotFound { - klog.Errorf("Failed to scrape timestamp from nb_Global table "+ - "err: %v", err) - return - } + if v, ok = options["e2e_timestamp"]; !ok { + klog.V(5).Infof("Failed to find 'e2e-timestamp' from %s options. This may occur at startup.", name) + return 0 + } - if nbGlobal != nil { - scrapeAndSetE2ETimestampMetrics(nbGlobal.Options) - } else { - metricDBE2eTimestamp.WithLabelValues(database).Set(0) - } + if value, err := strconv.ParseFloat(v, 64); err != nil { + klog.Errorf("Failed to parse 'e2e-timestamp' value to float64 err: %v", err) + return 0 + } else { + return value } } diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go index 36ac595432..073bc3d7dc 100644 --- a/go-controller/pkg/ovn/master.go +++ b/go-controller/pkg/ovn/master.go @@ -92,8 +92,7 @@ func (oc *Controller) Start(nodeName string, wg *sync.WaitGroup, ctx context.Con end := time.Since(start) metrics.MetricMasterReadyDuration.Set(end.Seconds()) }() - // run only on the active master node. - metrics.StartMasterMetricUpdater(oc.stopChan, oc.nbClient) + if err := oc.StartClusterMaster(nodeName); err != nil { panic(err.Error()) } @@ -215,6 +214,8 @@ func (oc *Controller) upgradeOVNTopology(existingNodes *kapi.NodeList) error { func (oc *Controller) StartClusterMaster(masterNodeName string) error { klog.Infof("Starting cluster master") + metrics.RunTimestamp(oc.stopChan, oc.sbClient, oc.nbClient) + metrics.MonitorIPSec(oc.nbClient) oc.metricsRecorder.Run(oc.sbClient) // enableOVNLogicalDataPathGroups sets an OVN flag to enable logical datapath From 77226773e38d44db7276186774a8352d4f2453bc Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Wed, 2 Mar 2022 17:15:23 +0000 Subject: [PATCH 16/25] Metrics: Split metrics registration leader vs non-leader We do not want to expose the majority of our metrics when we are not a leader. This PR registers most of the metrics only when leader. The metrics registration are grouped according to their task to aid code readability only. Signed-off-by: Martin Kennelly --- go-controller/cmd/ovnkube/ovnkube.go | 6 +- go-controller/pkg/metrics/master.go | 171 ++++++++++++++------------- go-controller/pkg/ovn/master.go | 2 + 3 files changed, 92 insertions(+), 87 deletions(-) diff --git a/go-controller/cmd/ovnkube/ovnkube.go b/go-controller/cmd/ovnkube/ovnkube.go index c46ec7434d..43f879c7a8 100644 --- a/go-controller/cmd/ovnkube/ovnkube.go +++ b/go-controller/cmd/ovnkube/ovnkube.go @@ -239,10 +239,8 @@ func runOvnKube(ctx *cli.Context) error { return fmt.Errorf("error when trying to initialize libovsdb SB client: %v", err) } - // register prometheus metrics exported by the master - // this must be done prior to calling controller start - // since we capture some metrics in Start() - metrics.RegisterMasterMetrics(libovsdbOvnNBClient, libovsdbOvnSBClient) + // register prometheus metrics that do not depend on becoming ovnkube-master leader + metrics.RegisterMasterBase() ovnController := ovn.NewOvnController(ovnClientset, masterWatchFactory, stopChan, nil, libovsdbOvnNBClient, libovsdbOvnSBClient, util.EventRecorder(ovnClientset.KubeClient)) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index c410610816..d60313c46b 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -245,92 +245,96 @@ var metricPortBindingUpLatency = prometheus.NewHistogram(prometheus.HistogramOpt Buckets: prometheus.ExponentialBuckets(.01, 2, 15), }) -var registerMasterMetricsOnce sync.Once - -// RegisterMasterMetrics registers some ovnkube master metrics with the Prometheus -// registry -func RegisterMasterMetrics(nbClient libovsdbclient.Client, sbClient libovsdbclient.Client) { - registerMasterMetricsOnce.Do(func() { - // ovnkube-master metrics - // the updater for this metric is activated - // after leader election - prometheus.MustRegister(MetricMasterLeader) - prometheus.MustRegister(metricPodCreationLatency) - prometheus.MustRegister(prometheus.NewCounterFunc( - prometheus.CounterOpts{ - Namespace: MetricOvnkubeNamespace, - Subsystem: MetricOvnkubeSubsystemMaster, - Name: "skipped_nbctl_daemon_total", - Help: "The number of times we skipped using ovn-nbctl daemon and directly interacted with OVN NB DB", - }, func() float64 { - return float64(util.SkippedNbctlDaemonCounter) - })) - prometheus.MustRegister(MetricMasterReadyDuration) - prometheus.MustRegister(metricOvnCliLatency) - // this is to not to create circular import between metrics and util package - util.MetricOvnCliLatency = metricOvnCliLatency - prometheus.MustRegister(MetricResourceUpdateCount) - prometheus.MustRegister(MetricResourceAddLatency) - prometheus.MustRegister(MetricResourceUpdateLatency) - prometheus.MustRegister(MetricResourceDeleteLatency) - prometheus.MustRegister(MetricRequeueServiceCount) - prometheus.MustRegister(MetricSyncServiceCount) - prometheus.MustRegister(MetricSyncServiceLatency) - prometheus.MustRegister(prometheus.NewGaugeFunc( - prometheus.GaugeOpts{ - Namespace: MetricOvnkubeNamespace, - Subsystem: MetricOvnkubeSubsystemMaster, - Name: "build_info", - Help: "A metric with a constant '1' value labeled by version, revision, branch, " + - "and go version from which ovnkube was built and when and who built it", - ConstLabels: prometheus.Labels{ - "version": "0.0", - "revision": config.Commit, - "branch": config.Branch, - "build_user": config.BuildUser, - "build_date": config.BuildDate, - "goversion": runtime.Version(), - }, +// RegisterMasterBase registers ovnkube master base metrics with the Prometheus registry. +// This function should only be called once. +func RegisterMasterBase() { + prometheus.MustRegister(MetricMasterLeader) + prometheus.MustRegister(MetricMasterReadyDuration) + prometheus.MustRegister(prometheus.NewGaugeFunc( + prometheus.GaugeOpts{ + Namespace: MetricOvnkubeNamespace, + Subsystem: MetricOvnkubeSubsystemMaster, + Name: "build_info", + Help: "A metric with a constant '1' value labeled by version, revision, branch, " + + "and go version from which ovnkube was built and when and who built it", + ConstLabels: prometheus.Labels{ + "version": "0.0", + "revision": config.Commit, + "branch": config.Branch, + "build_user": config.BuildUser, + "build_date": config.BuildDate, + "goversion": runtime.Version(), }, - func() float64 { return 1 }, - )) - prometheus.MustRegister(metricV4HostSubnetCount) - prometheus.MustRegister(metricV6HostSubnetCount) - prometheus.MustRegister(metricV4AllocatedHostSubnetCount) - prometheus.MustRegister(metricV6AllocatedHostSubnetCount) - prometheus.MustRegister(metricEgressIPCount) - prometheus.MustRegister(metricEgressFirewallRuleCount) - prometheus.MustRegister(metricEgressFirewallCount) - prometheus.MustRegister(metricEgressRoutingViaHost) - registerWorkqueueMetrics(MetricOvnkubeNamespace, MetricOvnkubeSubsystemMaster) - prometheus.MustRegister(prometheus.NewGaugeFunc( - prometheus.GaugeOpts{ - Namespace: MetricOvnNamespace, - Subsystem: MetricOvnSubsystemNorthd, - Name: "northd_probe_interval", - Help: "The maximum number of milliseconds of idle time on connection to the OVN SB " + - "and NB DB before sending an inactivity probe message", - }, func() float64 { - var nbGlobal *nbdb.NBGlobal - var probeInterval string - var ok bool - var err error - - if nbGlobal, err = libovsdbops.FindNBGlobal(nbClient); err != nil { - klog.Errorf("Failed to get NB_Global table "+ - "err: %v", err) - return 0 - } + }, + func() float64 { return 1 }, + )) +} - if probeInterval, ok = nbGlobal.Options["northd_probe_interval"]; !ok { - klog.Errorf("Failed to get northd_probe_interval from NB_Global table options column") - return 0 - } +// RegisterMasterPerformance registers metrics that help us understand ovnkube-master performance. Call once after LE is won. +func RegisterMasterPerformance(nbClient libovsdbclient.Client) { + // No need to unregister because process exits when leadership is lost. + prometheus.MustRegister(metricPodCreationLatency) + prometheus.MustRegister(MetricResourceUpdateCount) + prometheus.MustRegister(MetricResourceAddLatency) + prometheus.MustRegister(MetricResourceUpdateLatency) + prometheus.MustRegister(MetricResourceDeleteLatency) + prometheus.MustRegister(MetricRequeueServiceCount) + prometheus.MustRegister(MetricSyncServiceCount) + prometheus.MustRegister(MetricSyncServiceLatency) + prometheus.MustRegister(metricOvnCliLatency) + // This is set to not create circular import between metrics and util package + util.MetricOvnCliLatency = metricOvnCliLatency + registerWorkqueueMetrics(MetricOvnkubeNamespace, MetricOvnkubeSubsystemMaster) + prometheus.MustRegister(prometheus.NewCounterFunc( + prometheus.CounterOpts{ + Namespace: MetricOvnkubeNamespace, + Subsystem: MetricOvnkubeSubsystemMaster, + Name: "skipped_nbctl_daemon_total", + Help: "The number of times we skipped using ovn-nbctl daemon and directly interacted with OVN NB DB", + }, func() float64 { + return float64(util.SkippedNbctlDaemonCounter) + })) + prometheus.MustRegister(prometheus.NewGaugeFunc( + prometheus.GaugeOpts{ + Namespace: MetricOvnNamespace, + Subsystem: MetricOvnSubsystemNorthd, + Name: "northd_probe_interval", + Help: "The maximum number of milliseconds of idle time on connection to the OVN SB " + + "and NB DB before sending an inactivity probe message", + }, func() float64 { + var nbGlobal *nbdb.NBGlobal + var probeInterval string + var ok bool + var err error + + if nbGlobal, err = libovsdbops.FindNBGlobal(nbClient); err != nil { + klog.Errorf("Failed to get NB_Global table "+ + "err: %v", err) + return 0 + } - return parseMetricToFloat(MetricOvnSubsystemNorthd, "probe_interval", probeInterval) - }, - )) - }) + if probeInterval, ok = nbGlobal.Options["northd_probe_interval"]; !ok { + klog.Errorf("Failed to get northd_probe_interval from NB_Global table options column") + return 0 + } + + return parseMetricToFloat(MetricOvnSubsystemNorthd, "probe_interval", probeInterval) + }, + )) +} + +// RegisterMasterFunctional is a collection of metrics that help us understand ovnkube-master functions. Call once after +// LE is won. +func RegisterMasterFunctional() { + // No need to unregister because process exits when leadership is lost. + prometheus.MustRegister(metricV4HostSubnetCount) + prometheus.MustRegister(metricV6HostSubnetCount) + prometheus.MustRegister(metricV4AllocatedHostSubnetCount) + prometheus.MustRegister(metricV6AllocatedHostSubnetCount) + prometheus.MustRegister(metricEgressIPCount) + prometheus.MustRegister(metricEgressFirewallRuleCount) + prometheus.MustRegister(metricEgressFirewallCount) + prometheus.MustRegister(metricEgressRoutingViaHost) } // RunTimestamp adds a goroutine that registers and updates timestamp metrics. @@ -517,6 +521,7 @@ func NewControlPlaneRecorder() *ControlPlaneRecorder { } } +//Run will register the necessary metrics and add event handlers. func (ps *ControlPlaneRecorder) Run(sbClient libovsdbclient.Client) { // only register the metrics when we want them prometheus.MustRegister(metricFirstSeenLSPLatency) diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go index 073bc3d7dc..998d705b17 100644 --- a/go-controller/pkg/ovn/master.go +++ b/go-controller/pkg/ovn/master.go @@ -214,6 +214,8 @@ func (oc *Controller) upgradeOVNTopology(existingNodes *kapi.NodeList) error { func (oc *Controller) StartClusterMaster(masterNodeName string) error { klog.Infof("Starting cluster master") + metrics.RegisterMasterPerformance(oc.nbClient) + metrics.RegisterMasterFunctional() metrics.RunTimestamp(oc.stopChan, oc.sbClient, oc.nbClient) metrics.MonitorIPSec(oc.nbClient) oc.metricsRecorder.Run(oc.sbClient) From d364e23334dfcf6b6cfbcc22af73fe9e3d100722 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 3 Mar 2022 08:16:30 -0600 Subject: [PATCH 17/25] vendor: bump libovsdb to c8b4494412b1a0ba1396dd162a9a2d497c80f2b0 9598c132c2616 Fixes client error handling fc5e884e9f41a Moves client error handling from the cache to client 87acccaf29896 client: use LastTransactionID when the only monitor is a CondSince one 5b14611220b61 test: Using wait api with integration bb1929ac6d2ec Mapper: FieldByColumn error message shall contain table name ebe15cf84d8c4 Fix libovsdb server close 976fa4167e523 adding Wait to client api 6791b3ba1f1ce api: support pointer to slice of Models for List Signed-off-by: Dan Williams --- go-controller/go.mod | 2 +- go-controller/go.sum | 2 + .../ovn-org/libovsdb/cache/cache.go | 11 +-- .../github.com/ovn-org/libovsdb/client/api.go | 99 ++++++++++++++++++- .../ovn-org/libovsdb/client/client.go | 79 ++++++++++++--- .../ovn-org/libovsdb/mapper/info.go | 20 +++- .../ovn-org/libovsdb/ovsdb/condition.go | 6 ++ .../ovn-org/libovsdb/server/server.go | 18 +++- go-controller/vendor/modules.txt | 2 +- 9 files changed, 208 insertions(+), 31 deletions(-) diff --git a/go-controller/go.mod b/go-controller/go.mod index ca5372fa81..90981f1b04 100644 --- a/go-controller/go.mod +++ b/go-controller/go.mod @@ -21,7 +21,7 @@ require ( github.com/onsi/gomega v1.14.0 github.com/openshift/api v0.0.0-20211201215911-5a82bae32e46 github.com/openshift/client-go v0.0.0-20211202194848-d3f186f2d366 - github.com/ovn-org/libovsdb v0.6.1-0.20220127023511-a619f0fd93be + github.com/ovn-org/libovsdb v0.6.1-0.20220225160119-c8b4494412b1 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 github.com/spf13/afero v1.4.1 diff --git a/go-controller/go.sum b/go-controller/go.sum index c8b1e4b08d..977a0c645e 100644 --- a/go-controller/go.sum +++ b/go-controller/go.sum @@ -401,6 +401,8 @@ github.com/openshift/client-go v0.0.0-20211202194848-d3f186f2d366/go.mod h1:HJeH github.com/ory/dockertest/v3 v3.8.0/go.mod h1:9zPATATlWQru+ynXP+DytBQrsXV7Tmlx7K86H6fQaDo= github.com/ovn-org/libovsdb v0.6.1-0.20220127023511-a619f0fd93be h1:U8WVtNNTfBKj/5OE3uBe57oNJ+x5KSMl+3hM7iLSbdk= github.com/ovn-org/libovsdb v0.6.1-0.20220127023511-a619f0fd93be/go.mod h1:aLvY7gPs/vLyJXF+PpZzvWlR5LB4QNJvBYIQMskJLZk= +github.com/ovn-org/libovsdb v0.6.1-0.20220225160119-c8b4494412b1 h1:nz+mlIM2KFAJGhrKb68KBl36rFn2mLAp+kfskm2nfl0= +github.com/ovn-org/libovsdb v0.6.1-0.20220225160119-c8b4494412b1/go.mod h1:aLvY7gPs/vLyJXF+PpZzvWlR5LB4QNJvBYIQMskJLZk= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/cache/cache.go b/go-controller/vendor/github.com/ovn-org/libovsdb/cache/cache.go index 74b7bf085c..5199a4b319 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/cache/cache.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/cache/cache.go @@ -457,7 +457,6 @@ type TableCache struct { cache map[string]*RowCache eventProcessor *eventProcessor dbModel model.DatabaseModel - errorChan chan error ovsdb.NotificationHandler mutex sync.RWMutex logger *logr.Logger @@ -500,7 +499,6 @@ func NewTableCache(dbModel model.DatabaseModel, data Data, logger *logr.Logger) eventProcessor: eventProcessor, dbModel: dbModel, mutex: sync.RWMutex{}, - errorChan: make(chan error), logger: logger, }, nil } @@ -545,7 +543,6 @@ func (t *TableCache) Update(context interface{}, tableUpdates ovsdb.TableUpdates } if err := t.Populate(tableUpdates); err != nil { t.logger.Error(err, "during libovsdb cache populate") - t.errorChan <- NewErrCacheInconsistent(err.Error()) return err } return nil @@ -560,7 +557,6 @@ func (t *TableCache) Update2(context interface{}, tableUpdates ovsdb.TableUpdate } if err := t.Populate2(tableUpdates); err != nil { t.logger.Error(err, "during libovsdb cache populate2") - t.errorChan <- NewErrCacheInconsistent(err.Error()) return err } return nil @@ -677,7 +673,7 @@ func (t *TableCache) Populate2(tableUpdates ovsdb.TableUpdates2) error { modified := model.Clone(existing) err := t.ApplyModifications(table, modified, *row.Modify) if err != nil { - return fmt.Errorf("unable to apply row modifications: %v", err) + return fmt.Errorf("unable to apply row modifications: %w", err) } if !model.Equal(modified, existing) { logger.V(5).Info("updating row", "old", fmt.Sprintf("%+v", existing), "new", fmt.Sprintf("%+v", modified)) @@ -736,11 +732,6 @@ func (t *TableCache) Run(stopCh <-chan struct{}) { wg.Wait() } -// Errors returns a channel where errors that occur during cache propagation can be received -func (t *TableCache) Errors() <-chan error { - return t.errorChan -} - // newRowCache creates a new row cache with the provided data // if the data is nil, and empty RowCache will be created func newRowCache(name string, dbModel model.DatabaseModel, dataType reflect.Type) *RowCache { diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/client/api.go b/go-controller/vendor/github.com/ovn-org/libovsdb/client/api.go index 0297ff3be1..0ca3f9bb5c 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/client/api.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/client/api.go @@ -16,6 +16,7 @@ import ( type API interface { // List populates a slice of Models objects based on their type // The function parameter must be a pointer to a slice of Models + // Models can be structs or pointers to structs // If the slice is null, the entire cache will be copied into the slice // If it has a capacity != 0, only 'capacity' elements will be filled in List(ctx context.Context, result interface{}) error @@ -69,6 +70,10 @@ type ConditionalAPI interface { // Delete returns the Operations needed to delete the models selected via the condition Delete() ([]ovsdb.Operation, error) + + // Wait returns the operations needed to perform the wait specified + // by the until condition, timeout, row and columns based on provided parameters. + Wait(ovsdb.WaitCondition, *int, model.Model, ...interface{}) ([]ovsdb.Operation, error) } // ErrWrongType is used to report the user provided parameter has the wrong type @@ -104,7 +109,23 @@ func (a api) List(ctx context.Context, result interface{}) error { return &ErrWrongType{resultPtr.Type(), "Expected pointer to slice of valid Models"} } - table, err := a.getTableFromModel(reflect.New(resultVal.Type().Elem()).Interface()) + // List accepts a slice of Models that can be either structs or pointer to + // structs + var appendValue func(reflect.Value) + var m model.Model + if resultVal.Type().Elem().Kind() == reflect.Ptr { + m = reflect.New(resultVal.Type().Elem().Elem()).Interface() + appendValue = func(v reflect.Value) { + resultVal.Set(reflect.Append(resultVal, v)) + } + } else { + m = reflect.New(resultVal.Type().Elem()).Interface() + appendValue = func(v reflect.Value) { + resultVal.Set(reflect.Append(resultVal, reflect.Indirect(v))) + } + } + + table, err := a.getTableFromModel(m) if err != nil { return err } @@ -141,7 +162,7 @@ func (a api) List(ctx context.Context, result interface{}) error { // clone only the models that match the predicate m := model.Clone(row) - resultVal.Set(reflect.Append(resultVal, reflect.Indirect(reflect.ValueOf(m)))) + appendValue(reflect.ValueOf(m)) i++ } return nil @@ -406,6 +427,80 @@ func (a api) Delete() ([]ovsdb.Operation, error) { return operations, nil } +func (a api) Wait(untilConFun ovsdb.WaitCondition, timeout *int, model model.Model, fields ...interface{}) ([]ovsdb.Operation, error) { + var operations []ovsdb.Operation + + /* + Ref: https://datatracker.ietf.org/doc/html/rfc7047.txt#section-5.2.6 + + lb := &nbdb.LoadBalancer{} + condition := model.Condition{ + Field: &lb.Name, + Function: ovsdb.ConditionEqual, + Value: "lbName", + } + timeout0 := 0 + client.Where(lb, condition).Wait( + ovsdb.WaitConditionNotEqual, // Until + &timeout0, // Timeout + &lb, // Row (and Table) + &lb.Name, // Cols (aka fields) + ) + */ + + conditions, err := a.cond.Generate() + if err != nil { + return nil, err + } + + table, err := a.getTableFromModel(model) + if err != nil { + return nil, err + } + + info, err := a.cache.DatabaseModel().NewModelInfo(model) + if err != nil { + return nil, err + } + + var columnNames []string + if len(fields) > 0 { + columnNames = make([]string, 0, len(fields)) + for _, f := range fields { + colName, err := info.ColumnByPtr(f) + if err != nil { + return nil, err + } + columnNames = append(columnNames, colName) + } + } + + row, err := a.cache.Mapper().NewRow(info, fields...) + if err != nil { + return nil, err + } + rows := []ovsdb.Row{row} + + for _, condition := range conditions { + operation := ovsdb.Operation{ + Op: ovsdb.OperationWait, + Table: table, + Where: condition, + Until: string(untilConFun), + Columns: columnNames, + Rows: rows, + } + + if timeout != nil { + operation.Timeout = timeout + } + + operations = append(operations, operation) + } + + return operations, nil +} + // getTableFromModel returns the table name from a Model object after performing // type verifications on the model func (a api) getTableFromModel(m interface{}) (string, error) { diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/client/client.go b/go-controller/vendor/github.com/ovn-org/libovsdb/client/client.go index 1672c311c8..877c5f3f87 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/client/client.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/client/client.go @@ -21,6 +21,7 @@ import ( "github.com/go-logr/logr" "github.com/go-logr/stdr" "github.com/ovn-org/libovsdb/cache" + "github.com/ovn-org/libovsdb/mapper" "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/ovn-org/libovsdb/ovsdb/serverdb" @@ -95,11 +96,14 @@ type ovsdbClient struct { primaryDBName string databases map[string]*database + errorCh chan error stopCh chan struct{} disconnect chan struct{} shutdown bool shutdownMutex sync.Mutex + handlerShutdown *sync.WaitGroup + logger *logr.Logger } @@ -146,7 +150,9 @@ func newOVSDBClient(clientDBModel model.ClientDBModel, opts ...Option) (*ovsdbCl deferredUpdates: make([]*bufferedUpdate, 0), }, }, - disconnect: make(chan struct{}), + errorCh: make(chan error), + handlerShutdown: &sync.WaitGroup{}, + disconnect: make(chan struct{}), } var err error ovs.options, err = newOptions(opts...) @@ -277,6 +283,15 @@ func (o *ovsdbClient) connect(ctx context.Context, reconnect bool) error { for dbName, db := range o.databases { db.monitorsMutex.Lock() defer db.monitorsMutex.Unlock() + + // Purge entire cache if no monitors exist to update dynamically + if len(db.monitors) == 0 { + db.cache.Purge(db.model) + continue + } + + // Restart all monitors; each monitor will handle purging + // the cache if necessary for id, request := range db.monitors { err := o.monitor(ctx, MonitorCookie{DatabaseName: dbName, ID: id}, true, request) if err != nil { @@ -289,8 +304,15 @@ func (o *ovsdbClient) connect(ctx context.Context, reconnect bool) error { go o.handleDisconnectNotification() for _, db := range o.databases { - go o.handleCacheErrors(o.stopCh, db.cache.Errors()) - go db.cache.Run(o.stopCh) + o.handlerShutdown.Add(1) + eventStopChan := make(chan struct{}) + go o.handleClientErrors(eventStopChan) + o.handlerShutdown.Add(1) + go func(db *database) { + defer o.handlerShutdown.Done() + db.cache.Run(o.stopCh) + close(eventStopChan) + }(db) } o.connected = true @@ -371,8 +393,6 @@ func (o *ovsdbClient) tryEndpoint(ctx context.Context, u *url.URL) (string, erro return "", err } db.api = newAPI(db.cache, o.logger) - } else { - db.cache.Purge(db.model) } db.cacheMutex.Unlock() } @@ -580,6 +600,10 @@ func (o *ovsdbClient) update(params []json.RawMessage, reply *[]interface{}) err err = db.cache.Update(cookie.ID, updates) db.cacheMutex.RUnlock() + if err != nil { + o.errorCh <- err + } + return err } @@ -617,6 +641,10 @@ func (o *ovsdbClient) update2(params []json.RawMessage, reply *[]interface{}) er err = db.cache.Update2(cookie, updates) db.cacheMutex.RUnlock() + if err != nil { + o.errorCh <- err + } + return err } @@ -859,18 +887,22 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne var args []interface{} if monitor.Method == ovsdb.ConditionalMonitorSinceRPC { - // FIXME: We should pass the monitor.LastTransactionID here - // But that would require delaying clearing the cache until - // after the monitors have been re-established - the logic - // would also need to be different for monitor and monitor_cond - // as we must always clear the cache in that instance - args = ovsdb.NewMonitorCondSinceArgs(dbName, cookie, requests, emptyUUID) + // If we are reconnecting a CondSince monitor that is the only + // monitor, then we can use its LastTransactionID since it is + // valid (because we're reconnecting) and we can safely keep + // the cache intact (because it's the only monitor). + transactionID := emptyUUID + if reconnecting && len(db.monitors) == 1 { + transactionID = monitor.LastTransactionID + } + args = ovsdb.NewMonitorCondSinceArgs(dbName, cookie, requests, transactionID) } else { args = ovsdb.NewMonitorArgs(dbName, cookie, requests) } var err error var tableUpdates interface{} + var lastTransactionFound bool switch monitor.Method { case ovsdb.MonitorRPC: var reply ovsdb.TableUpdates @@ -885,6 +917,7 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne err = o.rpcClient.CallWithContext(ctx, monitor.Method, args, &reply) if err == nil && reply.Found { monitor.LastTransactionID = reply.LastTransactionID + lastTransactionFound = true } tableUpdates = reply.Updates default: @@ -917,6 +950,16 @@ func (o *ovsdbClient) monitor(ctx context.Context, cookie MonitorCookie, reconne db.cacheMutex.Lock() defer db.cacheMutex.Unlock() + + // On reconnect, purge the cache _unless_ the only monitor is a + // MonitorCondSince one, whose LastTransactionID was known to the + // server. In this case the reply contains only updates to the existing + // cache data, while otherwise it includes complete DB data so we must + // purge to get rid of old rows. + if reconnecting && (len(db.monitors) > 1 || !lastTransactionFound) { + db.cache.Purge(db.model) + } + if monitor.Method == ovsdb.MonitorRPC { u := tableUpdates.(ovsdb.TableUpdates) err = db.cache.Populate(u) @@ -1046,13 +1089,19 @@ func (o *ovsdbClient) watchForLeaderChange() error { return nil } -func (o *ovsdbClient) handleCacheErrors(stopCh <-chan struct{}, errorChan <-chan error) { +func (o *ovsdbClient) handleClientErrors(stopCh <-chan struct{}) { + defer o.handlerShutdown.Done() + var errColumnNotFound *mapper.ErrColumnNotFound + var errCacheInconsistent *cache.ErrCacheInconsistent + var errIndexExists *cache.ErrIndexExists for { select { case <-stopCh: return - case err := <-errorChan: - if errors.Is(err, &cache.ErrCacheInconsistent{}) || errors.Is(err, &cache.ErrIndexExists{}) { + case err := <-o.errorCh: + if errors.As(err, &errColumnNotFound) { + o.logger.V(3).Error(err, "error updating cache, DB schema may be newer than client!") + } else if errors.As(err, &errCacheInconsistent) || errors.As(err, &errIndexExists) { // trigger a reconnect, which will purge the cache // hopefully a rebuild will fix any inconsistency o.logger.V(3).Error(err, "triggering reconnect to rebuild cache") @@ -1078,6 +1127,8 @@ func (o *ovsdbClient) handleDisconnectNotification() { // close the stopCh, which will stop the cache event processor close(o.stopCh) o.metrics.numDisconnects.Inc() + // wait for client related handlers to shutdown + o.handlerShutdown.Wait() o.rpcMutex.Lock() if o.options.reconnect && !o.shutdown { o.rpcClient = nil diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/mapper/info.go b/go-controller/vendor/github.com/ovn-org/libovsdb/mapper/info.go index d1fcb382ec..8ac436c790 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/mapper/info.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/mapper/info.go @@ -7,6 +7,24 @@ import ( "github.com/ovn-org/libovsdb/ovsdb" ) +// ErrColumnNotFound is an error that can occur when the column does not exist for a table +type ErrColumnNotFound struct { + column string + table string +} + +// Error implements the error interface +func (e *ErrColumnNotFound) Error() string { + return fmt.Sprintf("column: %s not found in table: %s", e.column, e.table) +} + +func NewErrColumnNotFound(column, table string) *ErrColumnNotFound { + return &ErrColumnNotFound{ + column: column, + table: table, + } +} + // Info is a struct that wraps an object with its metadata type Info struct { // FieldName indexed by column @@ -25,7 +43,7 @@ type Metadata struct { func (i *Info) FieldByColumn(column string) (interface{}, error) { fieldName, ok := i.Metadata.Fields[column] if !ok { - return nil, fmt.Errorf("FieldByColumn: column %s not found in orm info", column) + return nil, NewErrColumnNotFound(column, i.Metadata.TableName) } return reflect.ValueOf(i.Obj).Elem().FieldByName(fieldName).Interface(), nil } diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/ovsdb/condition.go b/go-controller/vendor/github.com/ovn-org/libovsdb/ovsdb/condition.go index 1415521c57..783ac0f554 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/ovsdb/condition.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/ovsdb/condition.go @@ -7,6 +7,7 @@ import ( ) type ConditionFunction string +type WaitCondition string const ( // ConditionLessThan is the less than condition @@ -25,6 +26,11 @@ const ( ConditionIncludes ConditionFunction = "includes" // ConditionExcludes is the excludes condition ConditionExcludes ConditionFunction = "excludes" + + // WaitConditionEqual is the equal condition + WaitConditionEqual WaitCondition = "==" + // WaitConditionNotEqual is the not equal condition + WaitConditionNotEqual WaitCondition = "!=" ) // Condition is described in RFC 7047: 5.1 diff --git a/go-controller/vendor/github.com/ovn-org/libovsdb/server/server.go b/go-controller/vendor/github.com/ovn-org/libovsdb/server/server.go index 88c0e0328e..19c037a4b8 100644 --- a/go-controller/vendor/github.com/ovn-org/libovsdb/server/server.go +++ b/go-controller/vendor/github.com/ovn-org/libovsdb/server/server.go @@ -100,6 +100,16 @@ func (o *OvsdbServer) Serve(protocol string, path string) error { } } +func isClosed(ch <-chan struct{}) bool { + select { + case <-ch: + return true + default: + } + + return false +} + // Close closes the OvsdbServer func (o *OvsdbServer) Close() { o.readyMutex.Lock() @@ -107,9 +117,13 @@ func (o *OvsdbServer) Close() { o.readyMutex.Unlock() // Only close the listener if Serve() has been called if o.listener != nil { - o.listener.Close() + if err := o.listener.Close(); err != nil { + o.logger.Error(err, "failed to close listener") + } + } + if !isClosed(o.done) { + close(o.done) } - close(o.done) } // Ready returns true if a server is ready to handle connections diff --git a/go-controller/vendor/modules.txt b/go-controller/vendor/modules.txt index c965f0f38e..c69450fec8 100644 --- a/go-controller/vendor/modules.txt +++ b/go-controller/vendor/modules.txt @@ -232,7 +232,7 @@ github.com/openshift/client-go/cloudnetwork/informers/externalversions/cloudnetw github.com/openshift/client-go/cloudnetwork/informers/externalversions/cloudnetwork/v1 github.com/openshift/client-go/cloudnetwork/informers/externalversions/internalinterfaces github.com/openshift/client-go/cloudnetwork/listers/cloudnetwork/v1 -# github.com/ovn-org/libovsdb v0.6.1-0.20220127023511-a619f0fd93be +# github.com/ovn-org/libovsdb v0.6.1-0.20220225160119-c8b4494412b1 ## explicit; go 1.16 github.com/ovn-org/libovsdb/cache github.com/ovn-org/libovsdb/client From d826ed0431042af572e4a591d964cb8e7c26c38d Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 3 Mar 2022 09:14:43 -0600 Subject: [PATCH 18/25] libovsdbops: update tests for changes to libovsdb API Signed-off-by: Dan Williams --- go-controller/pkg/libovsdbops/meter_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go-controller/pkg/libovsdbops/meter_test.go b/go-controller/pkg/libovsdbops/meter_test.go index 2e21061171..09c1259060 100644 --- a/go-controller/pkg/libovsdbops/meter_test.go +++ b/go-controller/pkg/libovsdbops/meter_test.go @@ -57,6 +57,11 @@ func (c *MockConditionalAPI) Delete() ([]ovsdb.Operation, error) { return nil, nil } +// Wait is an empty mock method. +func (c *MockConditionalAPI) Wait(ovsdb.WaitCondition, *int, model.Model, ...interface{}) ([]ovsdb.Operation, error) { + return nil, nil +} + // MockLibOvsDbClient is a mock implementation of libovsdbclient.ConditionalAPI. type MockLibOvsDbClient struct { libovsdbclient.ConditionalAPI From d2b746425a16aaefae8a067a5d0d5b717762dbcd Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Mon, 6 Sep 2021 08:49:11 -0500 Subject: [PATCH 19/25] kube: bump kube API QPS to 50 In some scale tests we get client-side throttling. Signed-off-by: Dan Williams --- go-controller/pkg/util/kube.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go-controller/pkg/util/kube.go b/go-controller/pkg/util/kube.go index 3cada4ea5d..07378c3be0 100644 --- a/go-controller/pkg/util/kube.go +++ b/go-controller/pkg/util/kube.go @@ -88,8 +88,8 @@ func newKubernetesRestConfig(conf *config.KubernetesConfig) (*rest.Config, error if err != nil { return nil, err } - kconfig.QPS = 25 - kconfig.Burst = 25 + kconfig.QPS = 50 + kconfig.Burst = 50 // if all the clients are behind HA-Proxy, then on the K8s API server side we only // see the HAProxy's IP and we can't tell the actual client making the request. kconfig.UserAgent = fmt.Sprintf("%s/%s@%s (%s/%s) kubernetes/%s", From 0e5692ed23e8ed57840990ba16435c9d99043eb7 Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Thu, 3 Mar 2022 19:18:40 +0000 Subject: [PATCH 20/25] Metrics: Filter events before spawning goroutine. Previous to this change, we spawned a go routine without checking if the event is relevant. This will lead to excessive go routine creation. Follow on work is needed to add a queue. Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/master.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index d60313c46b..1941704465 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -531,10 +531,16 @@ func (ps *ControlPlaneRecorder) Run(sbClient libovsdbclient.Client) { sbClient.Cache().AddEventHandler(&cache.EventHandlerFuncs{ AddFunc: func(table string, model model.Model) { - go ps.AddPortBindingEvent(table, model) + if table != portBindingTable { + return + } + go ps.AddPortBindingEvent(model) }, UpdateFunc: func(table string, old model.Model, new model.Model) { - go ps.UpdatePortBindingEvent(table, old, new) + if table != portBindingTable { + return + } + go ps.UpdatePortBindingEvent(old, new) }, DeleteFunc: func(table string, model model.Model) { }, @@ -571,10 +577,7 @@ func (ps *ControlPlaneRecorder) AddLSPEvent(podUID kapimtypes.UID) { r.timestampType = logicalSwitchPort } -func (ps *ControlPlaneRecorder) AddPortBindingEvent(table string, m model.Model) { - if table != portBindingTable { - return - } +func (ps *ControlPlaneRecorder) AddPortBindingEvent(m model.Model) { var r *record now := time.Now() row := m.(*sbdb.PortBinding) @@ -597,10 +600,7 @@ func (ps *ControlPlaneRecorder) AddPortBindingEvent(table string, m model.Model) r.timestampType = portBinding } -func (ps *ControlPlaneRecorder) UpdatePortBindingEvent(table string, old, new model.Model) { - if table != portBindingTable { - return - } +func (ps *ControlPlaneRecorder) UpdatePortBindingEvent(old, new model.Model) { var r *record oldRow := old.(*sbdb.PortBinding) newRow := new.(*sbdb.PortBinding) From 77678fbe280a701351decded0a16017f4e7a18aa Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Thu, 3 Mar 2022 15:12:21 -0500 Subject: [PATCH 21/25] Fix cleaning VF representor ports Signed-off-by: Tim Rozet --- go-controller/pkg/node/healthcheck.go | 6 ++++-- go-controller/pkg/node/healthcheck_test.go | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/go-controller/pkg/node/healthcheck.go b/go-controller/pkg/node/healthcheck.go index 1fcbe32838..d6a71567bd 100644 --- a/go-controller/pkg/node/healthcheck.go +++ b/go-controller/pkg/node/healthcheck.go @@ -228,6 +228,10 @@ func checkForStaleOVSRepresentorInterfaces(nodeName string, wf factory.ObjectCac // Remove any stale representor ports for _, ifaceInfo := range interfaceInfos { + // ignore non-vf representor ports + if _, ok := ifaceInfo.Attributes["vf-netdev-name"]; !ok { + continue + } ifaceId, ok := ifaceInfo.Attributes["iface-id"] if !ok { klog.Warningf("iface-id attribute was not found for OVS interface %s. "+ @@ -235,8 +239,6 @@ func checkForStaleOVSRepresentorInterfaces(nodeName string, wf factory.ObjectCac continue } if _, ok := expectedIfaceIds[ifaceId]; !ok { - // TODO(adrianc): To make this more strict we can check if the interface is a VF representor - // interface via sriovnet. klog.Warningf("Found stale OVS Interface, deleting OVS Port with interface %s", ifaceInfo.Name) _, stderr, err := util.RunOVSVsctl("--if-exists", "--with-iface", "del-port", ifaceInfo.Name) if err != nil { diff --git a/go-controller/pkg/node/healthcheck_test.go b/go-controller/pkg/node/healthcheck_test.go index d6a2da5e11..e8b20a5641 100644 --- a/go-controller/pkg/node/healthcheck_test.go +++ b/go-controller/pkg/node/healthcheck_test.go @@ -118,9 +118,9 @@ var _ = Describe("Healthcheck tests", func() { // mock call to find OVS interfaces with non-empty external_ids:sandbox execMock.AddFakeCmd(&ovntest.ExpectedCmd{ Cmd: genFindInterfaceWithSandboxCmd(), - Output: "pod-a-ifc,sandbox=123abcfaa iface-id=a-ns_a-pod\n" + - "pod-b-ifc,sandbox=123abcfaa iface-id=b-ns_b-pod\n" + - "stale-pod-ifc,sandbox=123abcfaa iface-id=stale-ns_stale-pod\n", + Output: "pod-a-ifc,sandbox=123abcfaa iface-id=a-ns_a-pod vf-netdev-name=blah\n" + + "pod-b-ifc,sandbox=123abcfaa iface-id=b-ns_b-pod vf-netdev-name=blah\n" + + "stale-pod-ifc,sandbox=123abcfaa iface-id=stale-ns_stale-pod vf-netdev-name=blah\n", Err: nil, }) From 6e1c2f665930f28e377cffe174ed2e90dc865310 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 2 Mar 2022 22:57:00 +0100 Subject: [PATCH 22/25] Add more debug logs to addLogicalPort Signed-off-by: Surya Seetharaman --- go-controller/pkg/ovn/pods.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/go-controller/pkg/ovn/pods.go b/go-controller/pkg/ovn/pods.go index 3cfb6d794b..5b29d874f9 100644 --- a/go-controller/pkg/ovn/pods.go +++ b/go-controller/pkg/ovn/pods.go @@ -313,10 +313,13 @@ func (oc *Controller) addLogicalPort(pod *kapi.Pod) (err error) { return nil } + var libovsdbExecuteTime time.Duration + var podAnnoTime time.Duration // Keep track of how long syncs take. start := time.Now() defer func() { - klog.Infof("[%s/%s] addLogicalPort took %v", pod.Namespace, pod.Name, time.Since(start)) + klog.Infof("[%s/%s] addLogicalPort took %v, libovsdb time %v, annotation time: %v", + pod.Namespace, pod.Name, time.Since(start), libovsdbExecuteTime, podAnnoTime) }() logicalSwitch := pod.Spec.NodeName @@ -478,7 +481,10 @@ func (oc *Controller) addLogicalPort(pod *kapi.Pod) (err error) { klog.V(5).Infof("Annotation values: ip=%v ; mac=%s ; gw=%s\nAnnotation=%s", podIfAddrs, podMac, podAnnotation.Gateways, marshalledAnnotation) - if err = oc.kube.SetAnnotationsOnPod(pod.Namespace, pod.Name, marshalledAnnotation); err != nil { + annoStart := time.Now() + err = oc.kube.SetAnnotationsOnPod(pod.Namespace, pod.Name, marshalledAnnotation) + podAnnoTime = time.Since(annoStart) + if err != nil { return fmt.Errorf("failed to set annotation on pod %s: %v", pod.Name, err) } releaseIPs = false @@ -584,7 +590,9 @@ func (oc *Controller) addLogicalPort(pod *kapi.Pod) (err error) { allOps = append(allOps, ops...) } + transactStart := time.Now() results, err := libovsdbops.TransactAndCheckAndSetUUIDs(oc.nbClient, lsp, allOps) + libovsdbExecuteTime = time.Since(transactStart) if err != nil { return fmt.Errorf("could not perform creation or update of logical switch port %s - %+v", portName, err) From 4f49a9e089cd6e76954f94482c6db90b92644bb6 Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Fri, 4 Mar 2022 12:38:10 +0000 Subject: [PATCH 23/25] Metrics: Correct unclear metric description Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/master.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index 1941704465..f2cc4ff084 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -23,10 +23,10 @@ import ( klog "k8s.io/klog/v2" ) -// metricNbE2eTimestamp is the UNIX timestamp value set to NB DB. A corresponding metric -// 'sb_e2e_timestamp' for SB DB will contain the timestamp that was written to NB DB. -// This is registered within func RunTimestamp in order to allow gathering this metric on -// the fly when metrics are scraped. +// metricNbE2eTimestamp is the UNIX timestamp value set to NB DB. Northd will eventually copy this +// timestamp from NB DB to SB DB. The metric 'sb_e2e_timestamp' stores the timestamp that is +// read from SB DB. This is registered within func RunTimestamp in order to allow gathering this +// metric on the fly when metrics are scraped. var metricNbE2eTimestamp = prometheus.NewGauge(prometheus.GaugeOpts{ Namespace: MetricOvnkubeNamespace, Subsystem: MetricOvnkubeSubsystemMaster, From 155e9f8027136e80f9b44b7fb2515a96bcbfbabf Mon Sep 17 00:00:00 2001 From: Martin Kennelly Date: Fri, 4 Mar 2022 12:40:17 +0000 Subject: [PATCH 24/25] Metrics: Remove duplication of code to extract timestamp Signed-off-by: Martin Kennelly --- go-controller/pkg/metrics/master.go | 70 ++++++++++------------------- 1 file changed, 23 insertions(+), 47 deletions(-) diff --git a/go-controller/pkg/metrics/master.go b/go-controller/pkg/metrics/master.go index f2cc4ff084..4463ce967e 100644 --- a/go-controller/pkg/metrics/master.go +++ b/go-controller/pkg/metrics/master.go @@ -245,6 +245,11 @@ var metricPortBindingUpLatency = prometheus.NewHistogram(prometheus.HistogramOpt Buckets: prometheus.ExponentialBuckets(.01, 2, 15), }) +const ( + globalOptionsTimestampField = "e2e_timestamp" + globalOptionsProbeIntervalField = "northd_probe_interval" +) + // RegisterMasterBase registers ovnkube master base metrics with the Prometheus registry. // This function should only be called once. func RegisterMasterBase() { @@ -302,23 +307,7 @@ func RegisterMasterPerformance(nbClient libovsdbclient.Client) { Help: "The maximum number of milliseconds of idle time on connection to the OVN SB " + "and NB DB before sending an inactivity probe message", }, func() float64 { - var nbGlobal *nbdb.NBGlobal - var probeInterval string - var ok bool - var err error - - if nbGlobal, err = libovsdbops.FindNBGlobal(nbClient); err != nil { - klog.Errorf("Failed to get NB_Global table "+ - "err: %v", err) - return 0 - } - - if probeInterval, ok = nbGlobal.Options["northd_probe_interval"]; !ok { - klog.Errorf("Failed to get northd_probe_interval from NB_Global table options column") - return 0 - } - - return parseMetricToFloat(MetricOvnSubsystemNorthd, "probe_interval", probeInterval) + return getGlobalOptionsValue(nbClient, globalOptionsProbeIntervalField) }, )) } @@ -347,24 +336,15 @@ func RunTimestamp(stopChan <-chan struct{}, sbClient, nbClient libovsdbclient.Cl // Metric named sb_e2e_timestamp is the UNIX timestamp observed in SB DB. The value is read from the SB DB // cache when metrics HTTP endpoint is scraped. - scrapeOvnSbE2eTimestamp := func() float64 { - sbGlobal, err := libovsdbops.FindSBGlobal(sbClient) - if err != nil { - klog.Errorf("Failed to get global options for the SB_Global table: %v", err) - return 0 - } - if val, ok := sbGlobal.Options["e2e_timestamp"]; ok { - return parseMetricToFloat(MetricOvnkubeSubsystemMaster, "sb_e2e_timestamp", val) - } - return 0 - } prometheus.MustRegister(prometheus.NewGaugeFunc( prometheus.GaugeOpts{ Namespace: MetricOvnkubeNamespace, Subsystem: MetricOvnkubeSubsystemMaster, Name: "sb_e2e_timestamp", Help: "The current e2e-timestamp value as observed in the southbound database", - }, scrapeOvnSbE2eTimestamp)) + }, func() float64 { + return getGlobalOptionsValue(sbClient, globalOptionsTimestampField) + })) // Metric named e2e_timestamp is the UNIX timestamp observed in NB and SB DBs cache with the DB name // (OVN_Northbound|OVN_Southbound) set as a label. Updated every 30s. @@ -379,10 +359,12 @@ func RunTimestamp(stopChan <-chan struct{}, sbClient, nbClient libovsdbclient.Cl currentTime := time.Now().Unix() if setNbE2eTimestamp(nbClient, currentTime) { metricNbE2eTimestamp.Set(float64(currentTime)) + } else { + metricNbE2eTimestamp.Set(0) } - metricDbTimestamp.WithLabelValues(nbClient.Schema().Name).Set(getDbOptionsTimestamp(nbClient)) - metricDbTimestamp.WithLabelValues(sbClient.Schema().Name).Set(getDbOptionsTimestamp(sbClient)) + metricDbTimestamp.WithLabelValues(nbClient.Schema().Name).Set(getGlobalOptionsValue(nbClient, globalOptionsTimestampField)) + metricDbTimestamp.WithLabelValues(sbClient.Schema().Name).Set(getGlobalOptionsValue(sbClient, globalOptionsTimestampField)) case <-stopChan: return } @@ -649,7 +631,7 @@ func getPodUIDFromPortBinding(row *sbdb.PortBinding) kapimtypes.UID { // setNbE2eTimestamp return true if setting timestamp to NB global options is successful func setNbE2eTimestamp(ovnNBClient libovsdbclient.Client, timestamp int64) bool { // assumption that only first row is relevant in NB_Global table - options := map[string]string{"e2e_timestamp": fmt.Sprintf("%d", timestamp)} + options := map[string]string{globalOptionsTimestampField: fmt.Sprintf("%d", timestamp)} if err := libovsdbops.UpdateNBGlobalOptions(ovnNBClient, options); err != nil { klog.Errorf("Unable to update NB global options E2E timestamp metric err: %v", err) return false @@ -657,7 +639,7 @@ func setNbE2eTimestamp(ovnNBClient libovsdbclient.Client, timestamp int64) bool return true } -func getDbOptionsTimestamp(client libovsdbclient.Client) float64 { +func getGlobalOptionsValue(client libovsdbclient.Client, field string) float64 { var options map[string]string dbName := client.Schema().Name @@ -678,22 +660,16 @@ func getDbOptionsTimestamp(client libovsdbclient.Client) float64 { options = sbGlobal.Options } } - return extractOptionsTimestamp(options, dbName) -} - -func extractOptionsTimestamp(options map[string]string, name string) float64 { - var v string - var ok bool - if v, ok = options["e2e_timestamp"]; !ok { - klog.V(5).Infof("Failed to find 'e2e-timestamp' from %s options. This may occur at startup.", name) - return 0 - } - - if value, err := strconv.ParseFloat(v, 64); err != nil { - klog.Errorf("Failed to parse 'e2e-timestamp' value to float64 err: %v", err) + if v, ok := options[field]; !ok { + klog.V(5).Infof("Failed to find %q from %s options. This may occur at startup.", field, dbName) return 0 } else { - return value + if value, err := strconv.ParseFloat(v, 64); err != nil { + klog.Errorf("Failed to parse %q value to float64 err: %v", field, err) + return 0 + } else { + return value + } } } From 24215ca8766c0e376169b2fc3b000eaabd0349bf Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Fri, 4 Mar 2022 11:31:44 -0500 Subject: [PATCH 25/25] NP Retry: return error for ensureAddrSet Missed a place where we should be returning an error so NP create will be retried. Signed-off-by: Tim Rozet --- go-controller/pkg/ovn/policy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/ovn/policy.go b/go-controller/pkg/ovn/policy.go index dcaf81258f..1c7c928176 100644 --- a/go-controller/pkg/ovn/policy.go +++ b/go-controller/pkg/ovn/policy.go @@ -914,8 +914,8 @@ func (oc *Controller) createNetworkPolicy(np *networkPolicy, policy *knet.Networ if hasAnyLabelSelector(ingressJSON.From) { klog.V(5).Infof("Network policy %s with ingress rule %s has a selector", policy.Name, ingress.policyName) if err := ingress.ensurePeerAddressSet(oc.addressSetFactory); err != nil { - klog.Errorf(err.Error()) - continue + np.Unlock() + return err } // Start service handlers ONLY if there's an ingress Address Set oc.handlePeerService(policy, ingress, np) @@ -951,8 +951,8 @@ func (oc *Controller) createNetworkPolicy(np *networkPolicy, policy *knet.Networ if hasAnyLabelSelector(egressJSON.To) { klog.V(5).Infof("Network policy %s with egress rule %s has a selector", policy.Name, egress.policyName) if err := egress.ensurePeerAddressSet(oc.addressSetFactory); err != nil { - klog.Errorf(err.Error()) - continue + np.Unlock() + return err } }