Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli #766

JacobTanenbaum · 2020-08-27T15:07:35Z

move the enabling of hybrid overlay into the ConfigMap for ovnkube like
all new arguments should be

Signed-off-by: Jacob Tanenbaum jtanenba@redhat.com

bindata/network/ovn-kubernetes/004-config.yaml

danwinship · 2020-08-27T18:38:56Z

bindata/network/ovn-kubernetes/004-config.yaml

+    {{ if .OVNHybridOverlayVXLANPort}}
+    hybrid-overlay-vxlan-port="{{.OVNHybridOverlayVXLANPort}}"
+    {{ end }}
+{{ end  }}


also, use {{- if ... }} and {{- end }} everywhere. The - after the {{ makes it discard extra whitespace so that you don't end up with extra blank lines in the generated file (which doesn't really matter, but it looks nicer and it's easy...)

I did this except for the first one so that a new line would appear between the sections like we expect

pkg/network/ovn_kubernetes_test.go

JacobTanenbaum · 2020-08-31T14:09:00Z

/retest

JacobTanenbaum · 2020-08-31T18:19:59Z

/retest

JacobTanenbaum · 2020-09-01T13:47:50Z

/retest

bindata/network/ovn-kubernetes/004-config.yaml

pkg/network/ovn_kubernetes_test.go

danwinship · 2020-09-01T20:12:14Z

/retest
looks like some CI burp?

danwinship

just nitpicks left

pkg/network/ovn_kubernetes_test.go

JacobTanenbaum · 2020-09-02T14:49:53Z

@danwinship thanks for the quick review! I addressed your nits and changed all the setting of uint32 pointers to ptrToUint32()

JacobTanenbaum · 2020-09-02T19:22:56Z

/retest

danwinship · 2020-09-02T21:37:53Z

/lgtm
alas we can't really skip e2e-gcp-ovn on this one...

openshift-bot · 2020-09-02T21:55:36Z

/retest

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

openshift-bot · 2020-09-02T22:08:41Z

/retest

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

openshift-bot · 2020-09-02T23:00:39Z

/retest

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

openshift-bot · 2020-09-02T23:52:38Z

/retest

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

danwinship · 2020-09-17T11:11:04Z

/lgtm

openshift-ci-robot · 2020-09-17T11:11:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, JacobTanenbaum

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [danwinship]

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

openshift-bot · 2020-09-18T04:39:10Z

/retest

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

openshift-bot · 2020-09-18T04:52:15Z

/retest

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

openshift-bot · 2020-09-18T05:43:59Z

/retest

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

openshift-bot · 2020-09-18T08:33:06Z

/retest

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

openshift-bot · 2020-09-18T11:22:03Z

/retest

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

openshift-bot · 2020-09-18T12:39:29Z

/retest

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

openshift-bot · 2020-09-18T14:10:31Z

/retest

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

openshift-ci-robot · 2020-09-18T16:45:07Z

@JacobTanenbaum: This pull request references Bugzilla bug 1880490, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli

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

JacobTanenbaum · 2020-09-18T16:45:47Z

/bugzilla refresh

openshift-ci-robot · 2020-09-18T16:45:51Z

@JacobTanenbaum: This pull request references Bugzilla bug 1880490, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

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

JacobTanenbaum · 2020-09-18T16:46:35Z

/bugzilla refresh

openshift-ci-robot · 2020-09-18T16:46:40Z

@JacobTanenbaum: This pull request references Bugzilla bug 1880490, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

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

openshift-bot · 2020-09-18T19:35:30Z

/retest

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

JacobTanenbaum · 2020-09-18T19:55:37Z

/retest

openshift-bot · 2020-09-18T21:32:34Z

/retest

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

openshift-bot · 2020-09-18T22:24:37Z

/retest

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

openshift-bot · 2020-09-18T23:03:35Z

/retest

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

openshift-ci-robot · 2020-09-19T00:59:21Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-operator-with-custom-vxlan-port	`443c564`	link	`/test e2e-operator-with-custom-vxlan-port`

Full PR test history. Your PR dashboard.

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

openshift-bot · 2020-09-19T01:13:30Z

/retest

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

openshift-bot · 2020-09-19T01:26:30Z

/retest

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

openshift-ci-robot · 2020-09-19T02:40:18Z

@JacobTanenbaum: All pull requests linked via external trackers have merged:

openshift/cluster-network-operator#766

Bugzilla bug 1880490 has been moved to the MODIFIED state.

In response to this:

Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli

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

openshift-ci-robot requested review from abhat and squeed August 27, 2020 15:07

danwinship reviewed Aug 27, 2020

View reviewed changes

JacobTanenbaum force-pushed the configMap branch 4 times, most recently from a4ed25a to d594370 Compare August 31, 2020 14:00

JacobTanenbaum force-pushed the configMap branch from 2dd095f to 8d1bdfc Compare August 31, 2020 14:56

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 31, 2020

JacobTanenbaum force-pushed the configMap branch from 8d1bdfc to b393f2a Compare August 31, 2020 15:00

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 31, 2020

JacobTanenbaum force-pushed the configMap branch from b393f2a to 354c6a9 Compare August 31, 2020 16:33

danwinship reviewed Sep 1, 2020

View reviewed changes

bindata/network/ovn-kubernetes/004-config.yaml Show resolved Hide resolved

pkg/network/ovn_kubernetes_test.go Outdated Show resolved Hide resolved

pkg/network/ovn_kubernetes_test.go Show resolved Hide resolved

pkg/network/ovn_kubernetes_test.go Outdated Show resolved Hide resolved

JacobTanenbaum force-pushed the configMap branch from 354c6a9 to 07e0f0c Compare September 2, 2020 14:19

danwinship reviewed Sep 2, 2020

View reviewed changes

pkg/network/ovn_kubernetes_test.go Show resolved Hide resolved

pkg/network/ovn_kubernetes_test.go Outdated Show resolved Hide resolved

pkg/network/ovn_kubernetes_test.go Outdated Show resolved Hide resolved

JacobTanenbaum force-pushed the configMap branch from 07e0f0c to 2f217a8 Compare September 2, 2020 14:47

openshift-ci-robot assigned danwinship Sep 2, 2020

openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 2, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2020

JacobTanenbaum changed the title ~~change the hybrid overlay config to use the ConfigMap instead of cli~~ Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli Sep 18, 2020

openshift-ci-robot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Sep 18, 2020

openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels Sep 18, 2020

openshift-merge-robot merged commit fc433f0 into openshift:master Sep 19, 2020

Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli #766

Bug 1880490: change the hybrid overlay config to use the ConfigMap instead of cli #766

Conversation

JacobTanenbaum commented Aug 27, 2020

danwinship Aug 27, 2020

Choose a reason for hiding this comment

JacobTanenbaum Sep 1, 2020

Choose a reason for hiding this comment

JacobTanenbaum commented Aug 31, 2020

JacobTanenbaum commented Aug 31, 2020

JacobTanenbaum commented Sep 1, 2020

danwinship commented Sep 1, 2020

danwinship left a comment

Choose a reason for hiding this comment

JacobTanenbaum commented Sep 2, 2020

JacobTanenbaum commented Sep 2, 2020

danwinship commented Sep 2, 2020

openshift-bot commented Sep 2, 2020

openshift-bot commented Sep 2, 2020

openshift-bot commented Sep 2, 2020

openshift-bot commented Sep 2, 2020

danwinship commented Sep 17, 2020

openshift-ci-robot commented Sep 17, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

JacobTanenbaum commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

JacobTanenbaum commented Sep 18, 2020

openshift-ci-robot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

JacobTanenbaum commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-bot commented Sep 18, 2020

openshift-ci-robot commented Sep 19, 2020 • edited

openshift-bot commented Sep 19, 2020

openshift-bot commented Sep 19, 2020

openshift-ci-robot commented Sep 19, 2020

openshift-ci-robot commented Sep 19, 2020 •

edited