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

GCE: Handle a nil configuration file #1121

Merged
merged 1 commit into from
Aug 16, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions controllers/gce/controller/cluster_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,17 +212,28 @@ func (c *ClusterManager) GC(lbNames []string, nodePorts []backends.ServicePort)
}

func getGCEClient(config io.Reader) *gce.GCECloud {
allConfig, err := ioutil.ReadAll(config)
if err != nil {
glog.Fatalf("Error while reading entire config: %v", err)
getConfigReader := func() io.Reader { return nil }

if config != nil {
allConfig, err := ioutil.ReadAll(config)
if err != nil {
glog.Fatalf("Error while reading entire config: %v", err)
}
glog.V(2).Infof("Using cloudprovider config file:\n%v ", string(allConfig))

getConfigReader = func() io.Reader {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of redefining getConfigReader, you can just update the variable that it returns.

configReader := nil
getConfigReader := func() io.Reader { return configReader }
if config != nil {
 // Update configReader
}

Copy link
Contributor Author

@nicksardo nicksardo Aug 15, 2017

Choose a reason for hiding this comment

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

Not quite that simple. If we went this route, configReader would need to be reset for each loop iteration.

for {
   if config != nil {
       configReader = bytes.NewReader(allConfig)
   }
...
...
}

Copy link
Contributor

Choose a reason for hiding this comment

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

aah, got it.
Sorry for missing it the first time

return bytes.NewReader(allConfig)
}
} else {
glog.V(2).Infoln("No cloudprovider config file provided. Continuing with default values.")
}

// Creating the cloud interface involves resolving the metadata server to get
// an oauth token. If this fails, the token provider assumes it's not on GCE.
// No errors are thrown. So we need to keep retrying till it works because
// we know we're on GCE.
for {
cloudInterface, err := cloudprovider.GetCloudProvider("gce", bytes.NewReader(allConfig))
cloudInterface, err := cloudprovider.GetCloudProvider("gce", getConfigReader())
if err == nil {
cloud := cloudInterface.(*gce.GCECloud)

Expand Down