Skip to content
This repository has been archived by the owner on Nov 19, 2020. It is now read-only.

Server Side Apply #123

Closed
sbunce opened this issue Dec 17, 2019 · 12 comments
Closed

Server Side Apply #123

sbunce opened this issue Dec 17, 2019 · 12 comments

Comments

@sbunce
Copy link

sbunce commented Dec 17, 2019

GKE just released 1.16 to rapid. I'd like to use server-side-apply. Generally, I'd like to declare protobufs in go, then use the new serve-side-apply feature.

Are there any design choices blocking this working in this client? If not, what implementation work would be required to get this working. Maybe it's something I can help out with.

Thanks!

@ericchiang
Copy link
Owner

Do you have any links to the HTTP API for server side apply?

@sbunce
Copy link
Author

sbunce commented Dec 22, 2019

I'm unable to find the spec for it. I used kubectl to apply (to my gke 1.16 cluster) and turned on debugging.

kubectl apply --v=9 --server-side -f ~/Desktop/test.yaml

request.go:1017] Request Body: {"apiVersion":"v1","data":{"data":"testing"},"kind":"ConfigMap","metadata":{"name":"testing","namespace":"default"}}
round_trippers.go:423] curl -k -v -XPATCH  -H "User-Agent: kubectl/v1.17.0 (linux/amd64) kubernetes/70132b0" -H "Accept: application/json" -H "Content-Type: application/apply-patch+yaml" 'https://35.233.144.138/api/v1/namespaces/default/configmaps/testing?fieldManager=kubectl&force=false'
round_trippers.go:443] PATCH https://35.233.144.138/api/v1/namespaces/default/configmaps/testing?fieldManager=kubectl&force=false 201 Created in 63 milliseconds
round_trippers.go:449] Response Headers:
round_trippers.go:452]     Audit-Id: a84d4463-5429-4c40-afe3-0b74fe0effd0
round_trippers.go:452]     Cache-Control: no-cache, private
round_trippers.go:452]     Content-Type: application/json
round_trippers.go:452]     Content-Length: 456
round_trippers.go:452]     Date: Sat, 21 Dec 2019 23:35:22 GMT
request.go:1017] Response Body: {"kind":"ConfigMap","apiVersion":"v1","metadata":{"name":"testing","namespace":"default","selfLink":"/api/v1/namespaces/default/configmaps/testing","uid":"f5038146-a3d6-4b1e-82cc-eff548b61a6a","resourceVersion":"669","creationTimestamp":"2019-12-21T23:35:22Z","managedFields":[{"manager":"kubectl","operation":"Apply","apiVersion":"v1","time":"2019-12-21T23:35:22Z","fieldsType":"FieldsV1","fieldsV1":{"f:data":{"f:data":{}}}}]},"data":{"data":"testing"}}
configmap/testing serverside-applied

kubectl apply implementation

@sbunce
Copy link
Author

sbunce commented Dec 22, 2019

I started hacking around and I feel like I'm close.
https://github.com/ericchiang/k8s/pull/124/files
(implementation is rough)

It's saying the following.

couldn't apply resource: couldn't apply resource: kubernetes api: Failure 400 Incorrect version specified in apply patch. Specified patch version: , expected: v1

I think I need to set the apiVersion on the object I'm sending as part of the request body. I'm about to go drinking, so I thought I'd post my progress now. :)

@sbunce
Copy link
Author

sbunce commented Dec 22, 2019

Getting closer. I have a "hello world" service I'm deploying for testing. It consists of a configmap, deployment, autoscaler, and service. I'm printing the marshaled json before it's sent to the API server.

I was reading some other issues and it looks like it's a non-goal of this client to be able to marshal json. Looks like the API server supports some sort-of extra fancyness where fields can be multiple different types. Like with a resource.Quantity it accepts a string, integer, or float. That's currently where the code I have barfs.

I'm trying to figure out if the server-side-apply supports accepting the patch as protobuf now.

My high-level goal is to be able to declare all of my configuration in go, so I can parameterize it in go.

Updated the draft PR with the latest code. Code is a total mess, but at this point I'm just figuring out how to get this working, not focused on making the code nice.

// This one worked.
main.go:178] Running apply.
2019/12/22 12:53:32 {
  "apiVersion": "v1",
  "data": {
    "testing": "blabla"
  },
  "kind": "ConfigMap",
  "metadata": {
    "name": "hello",
    "namespace": "hello"
  }
}

// This one did not work because the json decoder on the API server
// expects special handling of the resource.Quantity.
main.go:178] Running apply.
2019/12/22 12:53:33 {
  "apiVersion": "apps/v1",
  "kind": "Deployment",
  "metadata": {
    "labels": {
      "app": "hello"
    },
    "name": "hello",
    "namespace": "hello"
  },
  "spec": {
    "selector": {
      "matchLabels": {
        "app": "hello"
      }
    },
    "template": {
      "metadata": {
        "labels": {
          "app": "hello"
        }
      },
      "spec": {
        "containers": [
          {
            "env": [
              {
                "name": "FLAG_WORK",
                "value": "100ms"
              }
            ],
            "image": "gcr.io/default-226502/hello:ebfabeca6aa4",
            "livenessProbe": {
              "handler": {
                "httpGet": {
                  "path": "/healthz",
                  "port": {
                    "intVal": 8080
                  }
                }
              },
              "initialDelaySeconds": 30,
              "periodSeconds": 30
            },
            "name": "hello",
            "ports": [
              {
                "containerPort": 8080
              }
            ],
            "readinessProbe": {
              "handler": {
                "httpGet": {
                  "path": "/healthz",
                  "port": {
                    "intVal": 8080
                  }
                }
              },
              "initialDelaySeconds": 5,
              "periodSeconds": 5
            },
            "resources": {
              "limits": {
                "cpu": {
                  "string": "1000m"
                },
                "memory": {
                  "string": "100Mi"
                }
              },
              "requests": {
                "cpu": {
                  "string": "10m"
                },
                "memory": {
                  "string": "10Mi"
                }
              }
            }
          }
        ]
      }
    }
  }
}
couldn't apply resource: couldn't apply resource: kubernetes api: Failure 500 failed to create typed patch object: errors:
  .spec.template.spec.containers[name="hello"].livenessProbe.handler: schema error: invalid atom: inlined
  .spec.template.spec.containers[name="hello"].ports: element 0: associative list with keys has an element that omits key field "protocol"
  .spec.template.spec.containers[name="hello"].readinessProbe.handler: schema error: invalid atom: inlined
  .spec.template.spec.containers[name="hello"].resources.limits.cpu: schema error: invalid atom: named type: io.k8s.apimachinery.pkg.api.resource.Quantity
  .spec.template.spec.containers[name="hello"].resources.limits.memory: schema error: invalid atom: named type: io.k8s.apimachinery.pkg.api.resource.Quantity
  .spec.template.spec.containers[name="hello"].resources.requests.cpu: schema error: invalid atom: named type: io.k8s.apimachinery.pkg.api.resource.Quantity
  .spec.template.spec.containers[name="hello"].resources.requests.memory: schema error: invalid atom: named type: io.k8s.apimachinery.pkg.api.resource.Quantity

@sbunce
Copy link
Author

sbunce commented Dec 22, 2019

I spend a lot of time searching for how to send the patch as protobuf. I didn't come up with anything.

I decided to ask the apply working group for advice. :)

https://groups.google.com/forum/#!topic/kubernetes-wg-apply/YdgP7a2gfjM

@apelisse
Copy link

Thanks for asking the group, we're looking into it. We'll provide a response soon :-)

@liggitt
Copy link

liggitt commented Dec 24, 2019

The intorstring and resource.Quantity values in your patch are not serialized correctly. They should be formatted like:
"cpu":"1000m"
and
"port":8080

@liggitt
Copy link

liggitt commented Dec 24, 2019

How are you creating the marshaled JSON?

@sbunce
Copy link
Author

sbunce commented Dec 24, 2019

liggitt,

I was attempting to encode with "encoding/json" which I have learned is not correct. During this issue is when I became aware of the special json encoding requirements that Kubernetes expects. Those special requirements increase serialization complexity. It looks like the best way to handle this encoding is to depend on kubernetes/api (1) which pulls in a ton of transitive dependencies. This client is about avoiding dependencies so it will be unable to use that package. It seems like the approach of this client is to stick with protobuf serialization, which is simpler, since protobufs are strictly about marshaling datastructures and aren't increasing serialization complexity to make human interaction with the serialized format easier.

[rant]
I believe we'll be in a much better world if we can declare and parameterize our Kubernetes configuration in our native languages and send that configuration (datastructures) directly to the API server which is now declarative (thanks to server-side-apply). This is why I'm so excited about the server-side-apply feature.
[/rant]

(Eric Chaing should feel free to correct me if I'm not fully understanding the goals of this client)

  1. https://github.com/kubernetes/api

@kwiesmueller
Copy link

So as server-side-apply currently can't support proto and proto to json won't generate a valid result, I don't think it's possible to use server-side apply with this client right now.

If your objects were generated based on the openapispec instead of proto I think they would be compatible with server-side-apply. But that doesn't sound like the scope of this client.

@sbunce
Copy link
Author

sbunce commented Dec 27, 2019

Thanks all for the help! I believe I understand now.

Conclusion is that it's technically infeasible for server side apply to support protobufs due to the requirement of patch to make a distinction between present and nil, vs not present.

Looks like this client can't support marshaling yaml, but we can give the user a choice to specify already-marshaled yaml. I like this idea Eric had here to add RawResource. I'll experiment with this in the context of server-side-apply.
#101 (comment)

@sbunce
Copy link
Author

sbunce commented Dec 27, 2019

I'm going to completely stop working on this.

Moving my configuration into go isn't working out well enough.

@sbunce sbunce closed this as completed Dec 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants