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

new v1.0.0 CRD's resources validator is not backward compatible #1410

Closed
yufengshan opened this issue Feb 5, 2020 · 4 comments
Closed

new v1.0.0 CRD's resources validator is not backward compatible #1410

yufengshan opened this issue Feb 5, 2020 · 4 comments
Assignees
Milestone

Comments

@yufengshan
Copy link

yufengshan commented Feb 5, 2020

After upgraded to seldon v1.0.0 (with latest CRD) from v0.51/v0.4. The operator report error as below. It seems the new CRD has stricter validation requirement in the sdep resources section. With the following changes, operator can successfully deploy the sdep.

         limits:
-          cpu: 4
+          cpu: "4"
           memory: 4Gi
         requests:
-          cpu: 1
+          cpu: "1"

This breaks the backward compatibility, engineers have to update their sdeps. Can someone take a look to make it backward compatible? Thanks.

============ error ==============

.predictors.componentSpecs.spec.containers.resources.limits.cpu in body must be of type string: \"integer\"\nspec.predictors.componentSpecs.spec.containers.resources.requests.cpu in body must be of t
ype string: \"integer\""}                                                                                                                                                                              
github.com/go-logr/zapr.(*zapLogger).Error                                                                                                                                                             
        /go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128                                                                                                                                         
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler                                                                                                                  
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:218                                                                                                    
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem                                                                                                               
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:192                                                                                                    
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker                                                                                                                            
        /go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:171                                                                                                    
k8s.io/apimachinery/pkg/util/wait.JitterUntil.func1                                                                                                                                                    
        /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:152                                                                                                   
k8s.io/apimachinery/pkg/util/wait.JitterUntil                                                                                                                                                          
        /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:153                                                                                                   
k8s.io/apimachinery/pkg/util/wait.Until                                                                                                                                                                
        /go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:88 ```
@yufengshan yufengshan added bug triage Needs to be triaged and prioritised accordingly labels Feb 5, 2020
@ukclivecox
Copy link
Contributor

The OpenAPISchema spec is generated automatically by Kubebuilder which generates the CRD yaml from the golang spec. The actual value is meant to be a string : see https://kubernetes.io/docs/tasks/configure-pod-container/assign-cpu-resource/

However, I agree kubectl allows one to use integers.

@ukclivecox ukclivecox removed the triage Needs to be triaged and prioritised accordingly label Feb 6, 2020
@ukclivecox ukclivecox added this to the 1.1 milestone Feb 6, 2020
@ukclivecox ukclivecox self-assigned this Feb 13, 2020
@ukclivecox
Copy link
Contributor

The value is a Quantity which is meant to be a string: see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.15/#quantity-resource-core
When a Quantity is parsed from a string...

However, kubectl does allow integer for values of this and seems to parse them to Quantities.

@ukclivecox
Copy link
Contributor

I agree this is a braking change but I think it may be hard to implement the kubectl functionality which is above and beyond what CRDs provide without some hacks to the OpenAPISchema which is as stated generated.

We could update the docs FAQs for this?

@ukclivecox ukclivecox removed the bug label Feb 14, 2020
@ukclivecox
Copy link
Contributor

This works with the current master code on 1.16. Please reopen if still an issue.
Tested with:

apiVersion: machinelearning.seldon.io/v1
kind: SeldonDeployment
metadata:
  name: seldon-model
spec:
  name: test-deployment
  predictors:
  - componentSpecs:
    - spec:
        containers:
        - image: seldonio/mock_classifier_rest:1.3
          name: classifier
          resources:
            requests:
              memory: 1Gi
              cpu: 1
            limits:
              memory: 1Gi
              cpu: 2
    graph:
      children: []
      endpoint:
        type: REST
      name: classifier
      type: MODEL
    name: example
    replicas: 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants