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

Document usage of intstr.IntOrString in the k8s tutorial #148

Closed
cueckoo opened this issue Jul 3, 2021 · 15 comments
Closed

Document usage of intstr.IntOrString in the k8s tutorial #148

cueckoo opened this issue Jul 3, 2021 · 15 comments
Labels

Comments

@cueckoo
Copy link
Collaborator

cueckoo commented Jul 3, 2021

Originally opened by @grantzvolsky in cuelang/cue#148

The kubernetes tutorial shows how to Extract CUE templates directly from Go source.

The generated Cue templates include intstr.

The question is how to use this struct with Cue.

How could one e.g. configure a service targetPort? Neither of the configurations below work.

 service "backend" spec ports: [{
   port:       80
   targetPort: "backend-http" // Error 1
   targetPort: intstr.FromString("backend-http") // Error 2
   targetPort: { Type: 1, IntVal: 0, StrVal: "backend-http" } // Yields invalid output
 }]

Error 1:

service."backend".spec.ports.0.targetPort: conflicting values IntOrString and "backend-http" (mismatched types struct and string):
    ./cue/backend-overlays-local.cue:10:15
    ./pkg/k8s.io/api/core/v1/types_go_gen.cue:4389:15
terminating because of errors

Error 2:

service."backend".spec.ports.0.targetPort: undefined field "FromString":
    ./cue/backend-overlays-local.cue:10:15
a

Invalid output (yaml dump):

  ports:
  - name: 80-http
    port: 80
    protocol: TCP
    targetPort:
      IntVal: 0
      StrVal: backend-http
      Type: 1

A possible workaround would be to define IntOrString as IntOrString :: int | string, but that would go against the instructions in the template ("// Code generated by cue get go. DO NOT EDIT.").

@cueckoo cueckoo added Documentation get go issues related to cue get go NeedsInvestigation labels Jul 3, 2021
@cueckoo cueckoo closed this as completed Jul 3, 2021
@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#148 (comment)

@grantzvolsky If you look at the generated definition of IntOrString it is simply _, or, anything goes. The current extractor cannot interpret (Unm|M)arshalJSON methods, so it has to assume it can be any type.

Even though one should not edit generated files, one can without harm define files alongside it with new definitions that get unified with the existing ones.

So in pkg/apimachinery/pkg/util/intstr include a file named, for instance, manual.cue with the following contents:

package intstr

IntOrString: int | string

As long as the the name does not end with _gen.cue, it will not be overwritten. So you can regenerate your K8s files, without fear of writing the manual definitions. If the manual definitions ever clash with the generated ones in the future, it will just result in an error that you would have to resolve. So this technique can also be used to guard against changes in expected definitions.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @grantzvolsky in cuelang/cue#148 (comment)

@mpvl

If you look at the generated definition of IntOrString it is simply _

That doesn't seem to be so in my case.

Following your instructions I get

$ cue vet ./...
service."backend".spec.ports.0.targetPort: field "IntOrString" declared as definition and regular field:
    ./pkg/k8s.io/apimachinery/pkg/util/intstr/intstr_go_gen.cue:16:16
    ./pkg/k8s.io/apimachinery/pkg/util/intstr/manual.cue:3:14
terminating because of errors

$ cue eval pkg/k8s.io/apimachinery/pkg/util/intstr/intstr_go_gen.cue
IntOrString :: {
    Type:   Type
    IntVal: int32
    StrVal: string
}
Type ::     int
enumType :: 0 | 1
Int ::      0
String ::   1

$ cue eval pkg/k8s.io/apimachinery/pkg/util/intstr/manual.cue
IntOrString: int | string


I'm using go 1.13.1 and cue 0.0.11.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#148 (comment)

Very strange indeed. I've tried to reproduce the error, using different versions including the one you mentioned, but for me it always generates IntOrString :: _.

Could you maybe provide more information, such as the OS, exact K8s commit you were extracting from?

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @grantzvolsky in cuelang/cue#148 (comment)

I'm on a different laptop now and the issue prevails. Below is a screencast demonstrating the problem. It was recorded in a fresh Asciinema (Ubuntu 18.04) docker container.

Step 1: docker run --rm -i -t -v `pwd`/installers:/installers asciinema/asciinema

Step 2-n: [skip 0:40-1:20 and 2:35-4:15] https://asciinema.org/a/qBMKNMdX3gsCLClmdvqNP5a0J

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @lizzabees in cuelang/cue#148 (comment)

i'm experiencing this behavior too and struggling to understand where it's breaking. i don't believe it's impacted by cue version -- i've tested with all versions back to 0.0.4 with no change in behavior. this is what i'm seeing (using 0.0.14):

i have one machine that generates the correct definition (IntOrString :: _). on this machine, k8s.io/apimachinery is on ref 461753078381c979582f217a28eb759ebee5295d

on other machines/VMs it generates the incorrect definition

 IntOrString :: {
    Type:   Type   @protobuf(1,varint,opt,name=type,casttype=Type)
    IntVal: int32  @protobuf(2,varint,opt,name=intVal)
    StrVal: string @protobuf(3,bytes,opt,name=strVal)
}

k8s.io/apimachinery is on ref 6eb29fdf75dcc3a9e33ec30ef32b25c923789dd1.

what i'm not understanding is that even if i check out the same revision of apimachinery as on the working machine and do a cue get go it still generates the latter definition. how does cue get go work? does it generate from the source in your GOPATH?

i'm also seeing similar discrepancies in generated definitions for other k8s resources (for instance, resource request/limits)

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#148 (comment)

@lizzabees you can try go get -u or go get pkg@ 6eb29fdf75dcc3a9e33ec30ef32b25c923789dd1 to update to a specific revision.

cue get go uses golang.org/x/tools/packages under the hood. So it generates from the available version in the GOPATH or go modules path or whatever supported package manager in use under the current path.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#148 (comment)

@lizzabees I looked at the differences between the versions and don't see any difference of importance.

@grantzvolsky: I followed your install almost to the step using a Docker ubuntu:18.04 image (I don't have your install scripts, but otherwise identical). I'm still getting the correct output. Wild.

What are the Go versions used on each of the machines?

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mpvl in cuelang/cue#148 (comment)

FYI, I've used go1.13.1 (the version Grant mentioned he used) and the latest CUE.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @grantzvolsky in cuelang/cue#148 (comment)

@mpvl I reduced the problem demonstration to a single Dockerfile:

FROM ubuntu:18.04

RUN apt update && apt install -y git gcc wget

WORKDIR /tmp/go
RUN wget https://dl.google.com/go/go1.13.1.linux-amd64.tar.gz
RUN tar -xvf go1.13.1.linux-amd64.tar.gz && mv go /opt/go
RUN ln -sfn /opt/go/bin/go /usr/local/bin/go
RUN ln -sfn /opt/go/bin/gofmt /usr/local/bin/gofmt

WORKDIR /tmp/cue
RUN wget https://github.com/cuelang/cue/releases/download/v0.0.15/cue_0.0.15_Linux_x86_64.tar.gz
RUN tar -xvf cue_0.0.15_Linux_x86_64.tar.gz && mv cue /opt/cue
RUN ln -sfn /opt/cue /usr/local/bin/cue

WORKDIR /example
RUN go get k8s.io/api/core/v1
RUN cue get go k8s.io/api/core/v1
RUN cat /example/pkg/k8s.io/apimachinery/pkg/util/intstr/intstr_go_gen.cue

When I run docker build -t tmp/test --no-cache ., step 15/15 yields

IntOrString :: {
	Type:   Type   @protobuf(1,varint,opt,name=type,casttype=Type)
	IntVal: int32  @protobuf(2,varint,opt,name=intVal)
	StrVal: string @protobuf(3,bytes,opt,name=strVal)
}

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @grantzvolsky in cuelang/cue#148 (comment)

It turns out that installing cue via go get -u cuelang.org/go/cmd/cue resolves this issue!

To reproduce: run docker build -t tmp/test --no-cache . with

Dockerfile

FROM ubuntu:18.04

RUN apt update && apt install -y git gcc wget

WORKDIR /tmp/go
RUN wget https://dl.google.com/go/go1.13.1.linux-amd64.tar.gz
RUN tar -xvf go1.13.1.linux-amd64.tar.gz && mv go /opt/go
RUN ln -sfn /opt/go/bin/go /usr/local/bin/go
RUN ln -sfn /opt/go/bin/gofmt /usr/local/bin/gofmt

RUN GOBIN=/usr/local/bin/ go get -u cuelang.org/go/cmd/cue

WORKDIR /example
RUN go get k8s.io/api/core/v1
RUN cue get go k8s.io/api/core/v1
RUN cat /example/pkg/k8s.io/apimachinery/pkg/util/intstr/intstr_go_gen.cue

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @santoshborse in cuelang/cue#148 (comment)

I was facing slightly similar issue with,

MaxSurge: intstr.FromInt(1),
MaxUnavailable: intstr.FromInt(0),

with error,

cannot use intstr.FromInt(0) (type intstr.IntOrString) as type *intstr.IntOrString in field value

It worked in this way

Strategy: appsv1.DeploymentStrategy{
  Type: "RollingUpdate",
    RollingUpdate: &appsv1.RollingUpdateDeployment{
      MaxSurge: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(1)},
      MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(0)},
    },
},

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @a-palchikov in cuelang/cue#148 (comment)

I'm consistently getting IntOrString generated as:

IntOrString :: {
	"Type": Type   @protobuf(1,varint,opt,name=type,casttype=Type)
	IntVal: int32  @protobuf(2,varint,opt,name=intVal)
	StrVal: string @protobuf(3,bytes,opt,name=strVal)
}

I've tried go1.12.17 and go1.13.9 and installing cue with go get to no avail.
Not sure I understand how it would be generated as _ - can someone shed some light on this?

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @a-palchikov in cuelang/cue#148 (comment)

It looks like it silently fails if cue is not installed with the expected module path of cuelang.org/go/cmd/cue here. The package errors should not be ignored and would've provided the hint:

-: cannot find package "cuelang.org/go/cmd/cue/cmd/interfaces" in any of:
	/path/to/go/go/src/cuelang.org/go/cmd/cue/cmd/interfaces (from $GOROOT)
	/path/to/go/src/cuelang.org/go/cmd/cue/cmd/interfaces (from $GOPATH)

The following seems to fix it but I'm not sure this is the correct fix:

import stdruntime "runtime"
// ...
func initInterfaces() error {
	_, file, _, _ := stdruntime.Caller(0)
	cfg := &packages.Config{
		Dir:  filepath.Join(filepath.Dir(file), "interfaces"),
		Mode: packages.LoadAllSyntax,
	}
	p, err := packages.Load(cfg, "cuelang.org/go/cmd/cue/cmd/interfaces")
	if err != nil {
		return err
	}

	for _, pp := range p {
		if len(pp.Errors) != 0 {
			return pp.Errors[0] // do not ignore package load errors
		}
	}
	// ... skipped
}

After that, it works as expected:

23:55 $ cue --verbose get go k8s.io/apimachinery/pkg/util/intstr
--- Package k8s.io/apimachinery/pkg/util/intstr
    IntOrString implements encoding/json.Marshaler; setting type to _

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @mgoodness in cuelang/cue#148 (comment)

Ran into this issue with v0.2.2 installed from Homebrew. Can confirm that go get -u cuelang.org/go/cmd/cue resolves it.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @myitcv in cuelang/cue#148 (comment)

This issue generally feels like a symptom of the the various issues I summarised in
#621 (comment). On the basis we have a clear path forward via #621 (comment), I'm going to close this issue effectively as a duplicate of #621 (preferring that issue because of the more complete commentary)

The short version however is:

  • cue go get will not require cuelang.org/go as a dependency
  • cue go get pkg will fail if pkg is not resolvable via go list. Generally speaking in a world of modules this will mean the user must do a go get pkg (or equivalent) first
  • cue go get will (likely) be renamed to cue import go to better reflect the change in behaviour

Please feel free to raise any further questions/issues over there, or in a separate issue if you think it a separate issue.

Thanks

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

No branches or pull requests

1 participant