Require cluster id for AWS #16331
Require cluster id for AWS #16331
Conversation
openshift-ci-robot
added
the
size/M
|
/test integration |
rrati
force-pushed
the
require-cluster-id-3.7
branch
from
e0a9308
to
adc5769
Compare
rrati
force-pushed
the
require-cluster-id-3.7
branch
from
adc5769
to
9b30e95
Compare
|
@rrati are my hopes too high thinking that this might fix https://bugzilla.redhat.com/show_bug.cgi?id=1491202 ? |
|
/approve |
openshift-merge-robot
added
the
approved
| @@ -362,6 +362,11 @@ func (gce *GCECloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut [] | |||
| return nameservers, srchOut | |||
| } | |||
|
|
|||
| // HasClusterID returns true if the cluster has a clusterID | |||
| func (gce *GCECloud) HasClusterID() bool { | |||
| return true | |||
There was a problem hiding this comment.
@rrati isn't GCE cloud using Initialize() to get the cluster id and this should return true only when the clusterID is really available?
There was a problem hiding this comment.
@mfojtik Yes, this really should be checking if the GCE Cluster ID is set or not. I'll log an issue upstream and correct it.
There was a problem hiding this comment.
@mfojtik Actually, looking at the usage of GCE ClusterID this function should be as is, ie always returning true.
|
@mfojtik I don't see this fixing https://bugzilla.redhat.com/show_bug.cgi?id=1491202 . In this case, the controller-manager would exit if the cloud provider isn't labelled unless they pass an argument to allow an untagged cluster. It won't do anything to actually set the ClusterID. |
|
@mfojtik I think this needs a lgtm label if it is approved for merging |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mfojtik, rrati The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
Automatic merge from submit-queue (batch tested with PRs 16443, 16331) |
Rather than trying to compensate for this in the master-config.yaml (where user set options and auto-set options will collide), if we know that we're going to need this information and we know that we'll never be able to auto-create this information, we should block the upgrade until the information is present. Doing this is just going to lead to confusion later about whether we can or can't automatically remove it after doing the same upgrade block in 3.8. I'd actually be ok pairing a still-broken default (for one release!), with blocking on upgrade. |
Backporting of:
kubernetes/kubernetes#48612
kubernetes/kubernetes#49215
Plus changes in master-controller startup to look for the flag to allow untagged clouds or exit.