Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hardcoded namespace in ovnkube.sh #4

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

danwinship
Copy link
Contributor

ovnkube.sh hardcodes the namespace to look for the ovnkube-master service in, causing the nodes to never come up in a cluster-network-operator-managed cluster.

Upstream we should probably fix it so that this comes from an environment variable rather than being hardcoded, so that we don't need to carry a patch.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 23, 2019
Copy link
Contributor

@pecameron pecameron left a comment

Choose a reason for hiding this comment

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

Could you add some comments around this change so that when we try to get this stuff upstream it will get noticed?

@danwinship
Copy link
Contributor Author

/retest

@pecameron
Copy link
Contributor

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2019
@pecameron pecameron merged commit 4c28f9c into openshift:master Apr 24, 2019
@danwinship danwinship deleted the fix-namespace branch April 24, 2019 18:20
@@ -149,7 +149,7 @@ wait_for_event () {
ready_to_start_node () {

# See if ep is available ...
ovn_master_host=$(kubectl --server=${K8S_APISERVER} --token=${k8s_token} --certificate-authority=${K8S_CACERT} get ep -n ovn-kubernetes ovnkube-master 2>/dev/null | grep 6642 | sed 's/:/ /' | awk '/ovnkube-master/{ print $2 }')
ovn_master_host=$(kubectl --server=${K8S_APISERVER} --token=${k8s_token} --certificate-authority=${K8S_CACERT} get ep -n openshift-ovn-kubernetes ovnkube-master 2>/dev/null | grep 6642 | sed 's/:/ /' | awk '/ovnkube-master/{ print $2 }')
Copy link
Contributor

Choose a reason for hiding this comment

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

Upstream is different here; shouldn't we be doing the namespace as a configmap value and subbing it into the script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, openshift-ovn-kubernetes is 2 months behind upstream right now. it will get ironed out eventually

openshift-merge-robot pushed a commit that referenced this pull request Mar 29, 2022
 Bug 2055379: [release-4.8] fixing network policy
Billy99 added a commit to Billy99/ovn-kubernetes that referenced this pull request Nov 2, 2022
Host pods were not detected properly
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. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants