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

Negative Defaults Values Not Supported #1634

Closed
donmstewart opened this issue Aug 11, 2020 · 13 comments · Fixed by #1787
Closed

Negative Defaults Values Not Supported #1634

donmstewart opened this issue Aug 11, 2020 · 13 comments · Fixed by #1787
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@donmstewart
Copy link

What broke?

Schema generation

What did you expect to happen? What do you think went wrong?

For int/int32/int64 have the ability to set negative default values using tags

// Set the maximum number of threads (both workers and masters) to be passed
// to TBB on initialization.  Generally speaking,
// 'max_tbb_threads_per_rank' - "1" TBB workers will be created.  
//
// Use "-1" for no limit.
//
// +optional
// +kubebuilder:default:=-1
MaxTbbThreadsPerRank int `json:"maxTbbThreadsPerRank,omitempty"`

Unfortunately it errors out with the following:-

Invalid value: "string": in body must be of type integer: "string"

Removing the default and the code generates as an integer.

Tried using kubebuilder:validation:Type:=int and with kubebuilder:validation:Format:=int32 as per https://swagger.io/docs/specification/data-models/data-types/

but unfortunately that does not seem to work either.

What versions of software are you using? Specifically, the following are often useful:

*** go version**

1.14.2

  • kubebuilder version

2.3.1

and scaffolding version (check your PROJECT file)
*** controller-runtime version**

v0.5.0

  • controller-tools version

v0.2.5

*** Kubernetes & kubectl versions**

1.18.6, 1.18.5

/kind bug

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 11, 2020
@camilamacedo86
Copy link
Member

camilamacedo86 commented Aug 15, 2020

I did not try to test it yet, however, see in the kubebuilder docs that the marker shows be:

// +kubebuilder:validation:Optional 
// +kubebuilder:validation:Type="<type>"
// +kubebuilder:validation:default=<value>

Which is NOT equals that markers used by you:

// +optional
// +kubebuilder:default:=-1

So, could you please check the Kubebuilder docs and try to follow up it? Also, if the problem still could you please check it with the master branch with the v3 plugin which will use an upper version of the controller-tools?

E.g:

kubebuilder init --plugins=go/v3-alpha --domain=tutorial.kubebuilder.io --project-version=3-alpha --repo=tutorial.kubebuilder.io/project --license apache2 --owner "The Kubernetes authors"

@donmstewart
Copy link
Author

Thanks for the follow up it's appreciated. I tried correcting the error you pointed out and alas still no go.

@gabbifish
Copy link
Contributor

gabbifish commented Sep 14, 2020

Hi @donmstewart! I haven't tried to reproduce this issue yet either, but just to make sure I know what behavior you're running into, are you still seeing

Invalid value: "string": in body must be of type integer: "string"

even after you've modified your annotation to look like the following?

// +kubebuilder:default=-1

Can you also try

// +kubebuilder:default="-1"

and let me know if that works?

@mjovanovic0
Copy link

Hi,

i stumbled upon same issue.

// +kubebuilder:default="-1"
ConnectionLimit int `json:"connectionLimit"`
// +kubebuilder:default=-1
ConnectionLimit int `json:"connectionLimit"`

will both produce:

connectionLimit:
  default: "-1"
  type: integer

While:

// +kubebuilder:default=0
ConnectionLimit int `json:"connectionLimit"`
// +kubebuilder:default=5
ConnectionLimit int `json:"connectionLimit"`

will produce:

connectionLimit:
  default: 0
  type: integer

connectionLimit:
  default: 5
  type: integer

@camilamacedo86
Copy link
Member

camilamacedo86 commented Sep 21, 2020

It shows a bug in the https://github.com/kubernetes-sigs/controller-tools. We need to check if it is not solved with https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.4.0. Could you try to bump the controller-gen in the Makefile (e.g https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v3/Makefile#L81) to use the version 0.4.0 and run make manifests to check if it is solved?

If not we need to fix it there.

@mjovanovic0
Copy link

Just tested and same issue with v0.4.0. -1 or "-1" both got generated as string literals.

Zero and positive numbers are correct.

@camilamacedo86
Copy link
Member

So, we only need to upgrade the controller-gen which will get in place with #1644.

@DirectXMan12
Copy link
Contributor

We fixed this bug in controller-tools recently: kubernetes-sigs/controller-tools@359a540

It's not in a release yet

@camilamacedo86 camilamacedo86 added this to the next milestone Nov 3, 2020
@camilamacedo86
Copy link
Member

camilamacedo86 commented Nov 3, 2020

Adding to the milestone next. It will be solved when we bump the new version with kubernetes-sigs/controller-tools@359a540

@camilamacedo86
Copy link
Member

camilamacedo86 commented Nov 5, 2020

Hi @donmstewart,

To solve this problem in your project so far you can upgrade the controller-gen version used by it to 0.4.0. However, note that it has breaking changes since for 0.4.0 the controller-tool generates CRD version to v1 by default (#468) and add admissionregisteration/v1 support for webhook (#469):

  • Update your makefile by replacing go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.3.0 ;\ for go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.0 ;\
  • Remove the controller-gen installed locally (check where it is by running which controller-gen)
  • Then, call the target that will download the specified version (make controller-gen or make manifests for example)

Also, note that with the PR #1787 the tool will start to build the projects with this upper version. However, it was addressed only for v3+ plugin/projects. To use v3+ and check it so far you need to:

  • Use kubebuilder from master branch (it is not released yet)
  • Create projects with the go/v3-alpha plugin via kubebuilder init --plugins=go/v3-alpha

Also, you can migrate your project by following the doc in the master branch. However, note that itis in an alpha stage. See: https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/migration/migration_guide_v2tov3.md

This issue will be closed when the PR #1787 get merged.

@camilamacedo86
Copy link
Member

It will be closed with : #1644

@donmstewart
Copy link
Author

@camilamacedo86 Thanks for the detailed notes, I will certainly give this a try.

@camilamacedo86
Copy link
Member

Closing this one. It is solved for v3-alpha plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment