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

Why does string representation matter depending on whether its imported from libsonnet file? #670

Closed
jlewi opened this issue Jun 28, 2018 · 3 comments
Milestone

Comments

@jlewi
Copy link

jlewi commented Jun 28, 2018

What happened:

I define the same service two different ways in the following two files

  1. I define it directly in the jsonnet file
  2. I define it an import it from the libsonnet file

https://github.com/jlewi/kubeflow-dev/blob/0a78fee4daace5a7adfcbf5efecb860a0bfd1f00/kubeflow-deployments/ks-app/components/test-central.jsonnet#L1
https://github.com/jlewi/kubeflow-dev/blob/0a78fee4daace5a7adfcbf5efecb860a0bfd1f00/kubeflow-deployments/ks-app/components/test-central.libsonnet#L1

The resulting service is different. In the case where it is defined in libsonnet

  annotations:
    getambassador.io/config: |-
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name: centralui-mapping
      prefix: /
      rewrite: /
      service: centraldashboard.kubeflow

When its defined in the jsonnet file

  annotations:
    getambassador.io/config: '---\napiVersion: ambassador/v0\nkind:  Mapping\nname:
      uiLocal-mapping\nprefix: /\nrewrite: /\nservice: uiLocal.kubeflow'

So the strong escaping appears to have changed and this appears to cause problems for Ambassador.

What you expected to happen:

I would expect it to always be

  annotations:
    getambassador.io/config: |-
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name: centralui-mapping
      prefix: /
      rewrite: /
      service: centraldashboard.kubeflow

How to reproduce it (as minimally and precisely as possible):

  1. Create a ksonnet app
  2. Copy the files listed above to your components directory
  3. ks show default -c test-central

Anything else we need to know?:

Environment:

  • ksonnet version (use ks version):
ksonnet version: 0.11.0
jsonnet version: v0.10.0
client-go version: 
  • Kubernetes version (use kubectl version):
Client Version: version.Info{Major:"1", Minor:"10", GitVersion:"v1.10.3", GitCommit:"2bba0127d85d5a46ab4b778548be28623b32d0b0", GitTreeState:"clean", BuildDate:"2018-05-21T09:17:39Z", GoVersion:"go1.9.3", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"9+", GitVersion:"v1.9.6-gke.1", GitCommit:"cb151369f60073317da686a6ce7de36abe2bda8d", GitTreeState:"clean", BuildDate:"2018-04-07T22:06:59Z", GoVersion:"go1.9.3b4", Compiler:"gc", Platform:"linux/amd64"}
jlewi added a commit to jlewi/kubeflow that referenced this issue Jun 28, 2018
* This is intended to support debugging IAP; we want to see what headers
  are on resulting requests.

* See kubeflow#574

* While creating this I ran into an issue with ksonnet not formatting the
  Ambassador mapping correctly unless we import it from a libsonnet file see
  ksonnet/ksonnet#670
k8s-ci-robot pushed a commit to kubeflow/kubeflow that referenced this issue Jun 29, 2018
* Create a version of echo-server to echo headers.

* This is intended to support debugging IAP; we want to see what headers
  are on resulting requests.

* See #574

* While creating this I ran into an issue with ksonnet not formatting the
  Ambassador mapping correctly unless we import it from a libsonnet file see
  ksonnet/ksonnet#670

* Address comments.

* Reference the images in kubeflow-images-public.

* Autoformat.
@bryanl bryanl added this to the 0.12.0 milestone Jul 1, 2018
bryanl added a commit to bryanl/ksonnet-lib that referenced this issue Jul 2, 2018
Re ksonnet/ksonnet#670

Signed-off-by: bryanl <bryanliles@gmail.com>
bryanl added a commit to bryanl/ksonnet-lib that referenced this issue Jul 2, 2018
Re ksonnet/ksonnet#670

Signed-off-by: bryanl <bryanliles@gmail.com>
@bryanl
Copy link
Member

bryanl commented Jul 2, 2018

@jlewi I noticed an interesting output from the Jsonnet parser. It will create strings that contain escaped control characters. I've updated the ksonnet jsonnet printer to account for this and will integrate this into ksonnet itself as well.

Output now looks like it should:

---
apiVersion: v1
kind: Service
metadata:
  annotations:
    getambassador.io/config: |-
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name: centralui-mapping
      prefix: /
      rewrite: /
      service: centraldashboard.libsonnet
  labels:
    app: centraldashboard
    ksonnet.io/component: test-central
  name: centraldashboard
  namespace: namespace
spec:
  ports:
  - port: 80
    targetPort: 8082
  selector:
    app: centraldashboard
  sessionAffinity: None
  type: ClusterIP
---
apiVersion: v1
kind: Service
metadata:
  annotations:
    getambassador.io/config: |-
      ---
      apiVersion: ambassador/v0
      kind:  Mapping
      name: uiLocal-mapping
      prefix: /
      rewrite: /
      service: uiLocal.jsonnet
  labels:
    app: centraldashboard
    ksonnet.io/component: test-central
  name: uiLocal
  namespace: kubeflow
spec:
  ports:
  - port: 80
    targetPort: 8082
  selector:
    app: centraldashboard
  sessionAffinity: None
  type: ClusterIP

bryanl added a commit to ksonnet/ksonnet-lib that referenced this issue Jul 2, 2018
Re ksonnet/ksonnet#670

Signed-off-by: bryanl <bryanliles@gmail.com>
@shomron shomron added this to In progress in Milestone 0.12.0 Jul 3, 2018
@shomron
Copy link
Collaborator

shomron commented Jul 3, 2018

Addressed via ksonnet/ksonnet-lib/pull/148, #682.

@shomron shomron closed this as completed Jul 3, 2018
Milestone 0.12.0 automation moved this from In progress to Done Jul 3, 2018
@jlewi
Copy link
Author

jlewi commented Jul 7, 2018

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

3 participants