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

improve kubectl error message when an object with no kind is passed to the resource builder #9357

Merged
merged 1 commit into from Jun 8, 2015

Conversation

mikedanese
Copy link
Member

Kindless api objects have their error message printed instead of glogged.

Fixes #9010

➜  kubernetes git:(kubectl-kind-err) ✗ cat bonkers/no-kind.yml 
apiVersion: v1beta3
metadata:
  labels:
    name: redis
    redis-sentinel: "true"
    role: master
  name: redis-master
spec:
  containers:
    - name: master
      image: kubernetes/redis:v1
      env:
        - name: MASTER
          value: "true"
      ports:
        - containerPort: 6379
      resources:
        limits:
          cpu: "1"
      volumeMounts:
        - mountPath: /redis-master-data
          name: data
    - name: sentinel
      image: kubernetes/redis:v1
      env:
        - name: SENTINEL
          value: "true"
      ports:
        - containerPort: 26379
  volumes:
    - name: data
      emptyDir: {}
➜  kubernetes git:(kubectl-kind-err) ✗ kubectl create -f bonkers/no-kind.yml
error: unable to load file "bonkers/no-kind.yml": kind not set in "bonkers/no-kind.yml"
error: no objects passed to create

Alternatively, we could not ContinueOnError by default. Any feelings @jlowdermilk @thockin?

@j3ffml
Copy link
Contributor

j3ffml commented Jun 6, 2015

It would make sense for the resource visitor to not ContinueOnError by default if there's only one object. @smarterclayton for input on that thought.

This change lgtm, though. It's a simple and definite improvement over the existing behavior.

@mikedanese mikedanese changed the title Kubectl kind err improve kubectl error message when an object with no kind is passed to the resource builder Jun 6, 2015
@smarterclayton
Copy link
Contributor

@jlowdermilk might make sense. We probably need to queue an issue to look at side loaded errors in visitor and see if there's an alternative to the way we have to aggressively continue loops even when errors happen (because someone might want to continueOnError). It might be a change to visitor to allow it to accept an error, like Go's path walker.

For now better to just report the message. Can we add a TODO in there though to revisit?

@@ -220,7 +220,7 @@ func (v *PathVisitor) Visit(fn VisitorFunc) error {
if !v.IgnoreErrors {
return err
}
glog.V(2).Infof("Unable to load file %q: %v", v.Path, err)
fmt.Printf("error: unable to load file %q: %v\n", v.Path, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be to os.Stderr

@bgrant0607
Copy link
Member

cc @ghodss

@mikedanese
Copy link
Member Author

@smarterclayton done.

@smarterclayton
Copy link
Contributor

LGTM

@smarterclayton smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2015
@mikedanese
Copy link
Member Author

Created #9405 as a spin off issue for the discussed error handling refactoring.

krousey added a commit that referenced this pull request Jun 8, 2015
improve kubectl error message when an object with no kind is passed to the resource builder
@krousey krousey merged commit 8ce921e into kubernetes:master Jun 8, 2015
@mikedanese mikedanese deleted the kubectl-kind-err branch June 8, 2015 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants