Skip to content
This repository was archived by the owner on Nov 9, 2022. It is now read-only.
This repository is currently being migrated. It's locked while the migration is in progress.

Check for api client before using #123

Merged
merged 1 commit into from
May 15, 2019
Merged

Check for api client before using #123

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 16 additions & 12 deletions pkg/controller/node/node_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
// Node controller errors.
var (
ErrCurrentClusterNotFound = errors.New("current cluster not found")
ErrNoAPIClient = errors.New("api client not available")
)

// Add creates a new Node Controller and adds it to the Manager. The Manager will set fields on the Controller
Expand Down Expand Up @@ -99,19 +100,18 @@ func (r *ReconcileNode) Reconcile(request reconcile.Request) (reconcile.Result,
return reconcileResult, err
}

// Set a storageos cluster client.
if r.stosClient != nil {
// Compare the cluster names, generations and UUIDs to check if it's
// the same cluster. Update the client if client cluster name,
// generation or UID are different from current cluster.
if r.stosClient.clusterName != cluster.GetName() ||
r.stosClient.clusterGeneration != cluster.GetGeneration() ||
r.stosClient.clusterUID != cluster.GetUID() {
r.setClientForCluster(cluster)
// Compare the cluster names, generations and UUIDs to check if it's
// the same cluster. Update the client if client cluster name,
// generation or UID are different from current cluster.
if r.stosClient == nil ||
r.stosClient.clusterName != cluster.GetName() ||
r.stosClient.clusterGeneration != cluster.GetGeneration() ||
r.stosClient.clusterUID != cluster.GetUID() {

if err := r.setClientForCluster(cluster); err != nil {
log.Println("failed to configure api client:", err)
return reconcileResult, err
Copy link
Contributor

Choose a reason for hiding this comment

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

This is much better 👍 return error and requeue the request.
I missed error checking 😅

}
} else {
// No previous client. Create a new client.
r.setClientForCluster(cluster)
}

// Sync labels to StorageOS node object.
Expand All @@ -130,6 +130,10 @@ func (r *ReconcileNode) syncLabels(name string, labels map[string]string) error
return nil
}

if r.stosClient == nil {
return ErrNoAPIClient
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. We need to add more tests for the functions in each of the controllers. pkg/controller/storageoscluster/storageoscluster_controller_test.go has some tests for the cluster controller.

// Get StorageOS node
node, err := r.stosClient.Node(name)
if err != nil {
Expand Down