Skip to content

OVN: use config file via ConfigMap rather than environment variables#333

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
pecameron:sdn456a
Dec 30, 2019
Merged

OVN: use config file via ConfigMap rather than environment variables#333
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
pecameron:sdn456a

Conversation

@pecameron
Copy link
Contributor

Configure ovnkube using a config file passed by the ovnkube-config
configMap and mountedinto the container.

SDN-456
https://jira.coreos.com/browse/SDN-456

Signed-off-by: Phil Cameron pcameron@redhat.com

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 4, 2019
@danwinship
Copy link
Contributor

You should squash #335 into this

@squeed
Copy link
Contributor

squeed commented Oct 7, 2019

This is a better way to do things, for sure. However, we need to add something to ovnkube that causes it to exit on configuration changes.

@danwinship
Copy link
Contributor

You should squash #335 into this

oops, too late. needs-rebase now

@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn-kubernetes

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 8, 2019
@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn-kubernetes

@squeed
Copy link
Contributor

squeed commented Oct 8, 2019

Two small changes, otherwise looks fine.

@pecameron
Copy link
Contributor Author

@squeed PTAL

@pecameron
Copy link
Contributor Author

/test e2e-aws
/test e2e-aws-ovn-kubernetes

@squeed
Copy link
Contributor

squeed commented Oct 9, 2019

Those test failures are not the expected set of failures for ovn-kube. Something is wrong. You'll have to dig in a bit further.

@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn-kubernetes

@pecameron
Copy link
Contributor Author

@squeed didn't spot anything (still not sure what to look for or where) so I ran the test again.

@squeed
Copy link
Contributor

squeed commented Oct 11, 2019

Huh... something is weird with the ingress operator:

"message": "pods \"ingress-operator-695f4b555-\" is forbidden: no SecurityContextConstraints found in cluster",

I don't think this is an ovn problem.

/test e2e-aws-ovn-kubernetes

@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn

2 similar comments
@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn

@dcbw
Copy link
Contributor

dcbw commented Dec 13, 2019

/test e2e-aws-ovn

@pecameron
Copy link
Contributor Author

/retest

@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn

@squeed
Copy link
Contributor

squeed commented Dec 16, 2019

It does seem like the MTU is problematic. Can you check to see if the MTU is correctly respected by all of the interfaces? Perhaps the gateway code hard-codes it when it shouldn't.

@squeed
Copy link
Contributor

squeed commented Dec 16, 2019

Yes, scanning the gateway code shows that it doesn't seem to consume mtu @dcbw

@pecameron
Copy link
Contributor Author

@squeed, @dcbw, whatever is happening here is not happening in other PRs. The mtu is generally handled properly. This might be a corner case. In the spirit of race conditions as we have seen in not getting the correct number of masters, is it possible that the sll certs are not set by the time they are used? The problem discussed in the f2f was in host networking ssl between ovs/ovn components.

@squeed
Copy link
Contributor

squeed commented Dec 16, 2019

My evidence for this is that the control plane seems to come up fine, but the openshift-apiserver (a non-hostnetwork component) times out when trying to write a response back to the kube-apsierver (which is hostnetwork).

Looking at the gateway code, I see that the interface created does not have the MTU passed.

Get what I'm saying? Would you mind checking to see if I'm right? If not, can you please spin up a cluster for me and I can check?

@pecameron
Copy link
Contributor Author

@squeed Yes I get what you are saying. Why does that happen in PR 333 and not happen in other PRs? What you are asserting should have wide spread impact.

The analysis from the f2f showed an ssl problem accessing sdb:9642 using host networking. The connection was established and ssl failed.

I am spinning up a cluster, I'll let you know when it is ready.

@pecameron
Copy link
Contributor Author

@squeed cluster didn't come up, bad token. Trying again.

Configure ovnkube using a config file passed by the ovnkube-config
configMap and mounted into the container.

SDN-498 Allow overriding MTU in ovn-kubernetes
https://jira.coreos.com/browse/SDN-498
The mtu is passed in the configmap.

bug 1779105
https://bugzilla.redhat.com/show_bug.cgi?id=1779105

SDN-456 OVN: use config file via ConfigMap rather than environment variables
https://jira.coreos.com/browse/SDN-456

Signed-off-by: Phil Cameron <pcameron@redhat.com>
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 16, 2019
@pecameron
Copy link
Contributor Author

/test e2e-aws-ovn

@pecameron
Copy link
Contributor Author

/retest
/test e2e-aws-ovn

@dcbw
Copy link
Contributor

dcbw commented Dec 29, 2019

/test e2e-aws-ovn
/test e2e-gcp

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Dec 29, 2019

@pecameron: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-ovn-kubernetes 247f1ad link /test e2e-aws-ovn-kubernetes

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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.

@dcbw
Copy link
Contributor

dcbw commented Dec 29, 2019

/test e2e-gcp

@dcbw
Copy link
Contributor

dcbw commented Dec 29, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 29, 2019
@dcbw
Copy link
Contributor

dcbw commented Dec 29, 2019

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dcbw, pecameron

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 29, 2019
@openshift-merge-robot openshift-merge-robot merged commit 1d08233 into openshift:master Dec 30, 2019
@pecameron pecameron deleted the sdn456a branch January 2, 2020 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants