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 create --validate, aggregate errors and handle requir… #10007

Merged

Conversation

feihujiang
Copy link
Contributor

improve kubectl create --validate, output aggregated errors and handle required fields

@k8s-bot
Copy link

k8s-bot commented Jun 18, 2015

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

@feihujiang
Copy link
Contributor Author

@satnam6502 @brendandburns @thockin Could you please review this PR?

@@ -102,11 +109,18 @@ func (s *SwaggerSchema) ValidateObject(obj interface{}, apiVersion, fieldName, t
}
fields, ok := obj.(map[string]interface{})
if !ok {
return fmt.Errorf("expected object of type map[string]interface{} as value of %s field", fieldName)
return append(allErrs, fmt.Errorf("expected object of type map[string]interface{} as value of %s field", fieldName))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not report the actual type with %T ?

@feihujiang
Copy link
Contributor Author

@satnam6502 OK, I changed according to your advice. Could you please review this PR again?

@satnam6502 satnam6502 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 18, 2015
@satnam6502
Copy link
Contributor

Please can you add a risk assessment for this PR? And link to a relevant bug? Thanks.

@satnam6502
Copy link
Contributor

ok to test

@satnam6502
Copy link
Contributor

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Jun 18, 2015

GCE e2e build/test failed for commit d5fcd9f.

@satnam6502
Copy link
Contributor

Retesting. If this fails again then please can someone investigate? Thanks.
@k8s-bot ok to test

@satnam6502
Copy link
Contributor

@feihujiang please can you link this to an issue?

@satnam6502
Copy link
Contributor

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Jun 18, 2015

GCE e2e build/test failed for commit d5fcd9f.

@satnam6502
Copy link
Contributor

@feihujiang please can you look at why e2e fails?

@satnam6502 satnam6502 self-assigned this Jun 18, 2015
@feihujiang
Copy link
Contributor Author

#10079 @satnam6502 I post a new issues, and link this PR to it.

@feihujiang
Copy link
Contributor Author

@satnam6502 When I opened Jenkins GCE e2e Details, it says "Access not possible". I think I have no right to open the Details now. Could you please post the Jenkins GCE e2e Details for me? Thanks!

@satnam6502
Copy link
Contributor

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Jun 19, 2015

GCE e2e build/test passed for commit d5fcd9f.

@satnam6502 satnam6502 added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jun 19, 2015
@satnam6502
Copy link
Contributor

e2e passes now. Will wait for issue to be triaged to work out milestone.

@satnam6502
Copy link
Contributor

Can you copy and paste some sample output? Thanks.

@feihujiang
Copy link
Contributor Author

@satnam6502 OK

The Invalid Pod:

   apiVersion: v1beta3
   kind: Pod
   metadata:
       labels:
           name: cassandra
       name: cassandra
   spec:
      containers:
      - args:
          - /run.sh
          resources:
            limits:
              cpu: "1"      
          name: cassandra
          ports:
          - name: cql
          - name: thrift
             containerPort: 9160
          volumeMounts:
          - name: data
            mountPath: /cassandra_data
          env:
          - name: MAX_HEAP_SIZE
            value: 512M
          - name: HEAP_NEWSIZE
            value: 100M
      volumes:
          - name: data
            emptyDir: {}

kubectl create --validate -f ./v1beta3/invalidCassandra.yaml
E0619 03:36:32.260674 14617 schema.go:153] Validation failed for: ports, [map[name:cql] map[containerPort:9160 name:thrift]]
E0619 03:36:32.260773 14617 schema.go:153] Validation failed for: containers, [map[volumeMounts:[map[mountPath:/cassandra_data name:data]] args:[/run.sh] env:[map[name:MAX_HEAP_SIZE value:512M] map[value:100M name:HEAP_NEWSIZE]] name:cassandra ports:[map[name:cql] map[containerPort:9160 name:thrift]] resources:map[limits:map[cpu:1]]]]
error validating "./v1beta3/invalidCassandra.yaml": error validating data: [ field image: is required, field containerPort: is required]

@satnam6502
Copy link
Contributor

Thank you.

@satnam6502
Copy link
Contributor

I will wait for the appropriate TL / meeting to determine the milestone for the associated issue before deciding what to do re: merging.

satnam6502 added a commit that referenced this pull request Jun 19, 2015
improve kubectl create --validate, aggregate errors and handle requir…
@satnam6502 satnam6502 merged commit f02c685 into kubernetes:master Jun 19, 2015
@feihujiang feihujiang deleted the improveCreateSchemaValidate branch November 24, 2015 07:19
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. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants