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

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

Closed
grantzvolsky opened this issue Oct 6, 2019 · 16 comments
Closed

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

grantzvolsky opened this issue Oct 6, 2019 · 16 comments
Labels

Comments

@grantzvolsky
Copy link

grantzvolsky commented Oct 6, 2019

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.").

@mpvl
Copy link
Contributor

mpvl commented Oct 7, 2019

@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.

@grantzvolsky
Copy link
Author

@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.

@mpvl
Copy link
Contributor

mpvl commented Oct 23, 2019

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?

@grantzvolsky
Copy link
Author

grantzvolsky commented Nov 6, 2019

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

@lizzabees
Copy link

lizzabees commented Nov 12, 2019

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)

@mpvl
Copy link
Contributor

mpvl commented Nov 30, 2019

@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.

@mpvl
Copy link
Contributor

mpvl commented Nov 30, 2019

@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?

@mpvl
Copy link
Contributor

mpvl commented Nov 30, 2019

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

@grantzvolsky
Copy link
Author

grantzvolsky commented Dec 7, 2019

@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)
}

@grantzvolsky
Copy link
Author

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

@santoshborse
Copy link

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)},
    },
},

@a-palchikov
Copy link

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?

@a-palchikov
Copy link

a-palchikov commented Apr 12, 2020

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 _

@mgoodness
Copy link

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

@mpvl mpvl added the get go issues related to cue get go label Jan 19, 2021
@myitcv
Copy link
Contributor

myitcv commented Jan 19, 2021

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

@cueckoo
Copy link

cueckoo commented Jul 3, 2021

This issue has been migrated to cue-lang/cue#148.

For more details about CUE's migration to a new home, please see cue-lang/cue#1078.

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

No branches or pull requests

8 participants