Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Aug 23, 2019

Proxy controller (I think) changed this openshift/cluster-network-operator#295 but the installer isn't doing the same. This is causing a different proxy config being generated between bootstrap and in-cluster which in turn causes the MCO to generates different machineconfigs and resulting in https://bugzilla.redhat.com/show_bug.cgi?id=1744532#c13

Signed-off-by: Antonio Murdaca runcom@linux.com

Signed-off-by: Antonio Murdaca <runcom@linux.com>
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 23, 2019
@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

/retest

@sdodson
Copy link
Member

sdodson commented Aug 23, 2019

This list still differs from the list in cluster-network-operator, do they need to be exactly the same?

@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

So from investigating that bug linked I noticed that those two were the hostnames missing, can't speak for the other differences between the lists but if they differs between bootstrap and in-cluster the MCO cannot proceed.

@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

Another question would be why this is only noticed on vsphere upi and not on aws ipi - not knowledgeable enough for that

@sdodson
Copy link
Member

sdodson commented Aug 23, 2019

Ok, I agree those should be added here. Just seemed like you were suggesting the lists needed to be identical to avoid problems.
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: runcom, sdodson

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 Aug 23, 2019
@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

Ok, I agree those should be added here. Just seemed like you were suggesting the lists needed to be identical to avoid problems.

so I'm not entirely sure about the list being identical :) just from debugging, .svc and .cluster.local are indeed missing from the noProxy list that the installer generates at bootstrap. Then, when the bootstrap is over, the MCO gets a proxy object that does have those 2 additional hostnames causing a skew and the subsequent MCO wedge.
Thanks!

@runcom runcom changed the title proxy: add .svc and .cluster.local to default noProxy Bug 1744532: proxy: add .svc and .cluster.local to default noProxy Aug 23, 2019
@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 23, 2019
@openshift-ci-robot
Copy link
Contributor

@runcom: This pull request references Bugzilla bug 1744532, 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.

Details

In response to this:

Bug 1744532: proxy: add .svc and .cluster.local to default noProxy

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.

@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@wking
Copy link
Member

wking commented Aug 23, 2019

... but if they differs between bootstrap and in-cluster the MCO cannot proceed.

So how do we ratchet future adjustments in without breaking anything? Ideally the installer-generated defaults would just be enough for the installer to be able to reach the registry to pull the release image and dependencies. Then we'd run the networking operator to (re-)render the manifest before pushing into the cluster to stay in sync?

@runcom
Copy link
Member Author

runcom commented Aug 23, 2019

So how do we ratchet future adjustments in without breaking anything?

so, one of the mystery here is that, apparently, this only happened on vsphere and hasn't happened on aws api so I think our current CI should have caught this just fine.

Ideally the installer-generated defaults would just be enough for the installer to be able to reach the registry to pull the release image and dependencies. Then we'd run the networking operator to (re-)render the manifest before pushing into the cluster to stay in sync?

we could do this yeah, right now, the installer has to keep up with networking operator changes as we've spotted here.

@openshift-bot
Copy link
Contributor

/retest

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 0454021 into openshift:master Aug 23, 2019
@openshift-ci-robot
Copy link
Contributor

@runcom: All pull requests linked via external trackers have merged. Bugzilla bug 1744532 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1744532: proxy: add .svc and .cluster.local to default noProxy

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.

@runcom runcom deleted the proxy-fix branch August 23, 2019 15:34
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. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged. 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.

6 participants