Fix up comments and errors around CRD issues #499
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bcreane
approved these changes
Aug 22, 2017
ozdanborne
force-pushed
the
better-crd-error
branch
from
8535cd7
to
c2cf0ec
Compare
gunjan5
reviewed
Aug 22, 2017
lib/backend/k8s/k8s.go
Outdated
| // as a backend but can often fail when using KDD as it relies | ||
| // on various custom resources existing. | ||
| // To ensure the datastore is initialized, this function checks that a | ||
| // known custom resource is defined: FelixGlobalConfigSetting. It accomplishes this |
There was a problem hiding this comment.
do you mean GlobalFelixConfig here?
lib/backend/k8s/k8s.go
Outdated
| // on various custom resources existing. | ||
| // To ensure the datastore is initialized, this function checks that a | ||
| // known custom resource is defined: FelixGlobalConfigSetting. It accomplishes this | ||
| // by trying to set the ClusterType (an example of FelixGlobalConfigSetting). |
There was a problem hiding this comment.
not sure I understand what you mean by an example of FelixGlobalConfigSetting. ClusterType is an instance of GlobalFelixConfig CRD
lib/backend/k8s/k8s.go
Outdated
| err := c.waitForClusterType() | ||
| if err != nil { | ||
| return fmt.Errorf("Failed to ensure ClusterType is set: %s", err) | ||
| return fmt.Errorf("Failed to ensure datastore has been initialized: \"%s\". Have you created custom resources and is calico authorized to access them?", err) |
There was a problem hiding this comment.
It's a bit odd to see a ? in an error message. How about something like: `return fmt.Errorf("Failed to ensure datastore has been initialized: "%s". Make sure the Custom Resource Definitions are created and Calico is authorized to access them", err)
ozdanborne
force-pushed
the
better-crd-error
branch
from
c2cf0ec
to
70f6894
Compare
ozdanborne
force-pushed
the
better-crd-error
branch
from
70f6894
to
99611e7
Compare
gunjan5
approved these changes
Aug 22, 2017
|
JGOC |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provide better comments and error messages around the code that checks the datastore is initialized. This error should help alleviate a possibly common scenario where users launch Calico in KDD mode without first creating the custom resources. In that scenario, we were seeing the following error in calico/node:
We should now see a more appropriate error:
ERROR: Unable to set node resource configuration: Failed to ensure the datastore has been initialized: "timed out waiting for the condition". Make sure the Custom Resource Definitions have been created and Calico has been authorized to access them.