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

Verify kubernetesVersion when run 'package verify'. #1296

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

harryge00
Copy link
Contributor

@harryge00 harryge00 commented Jan 21, 2020

What this PR does / why we need it:

Fix #1266.
But currently, we do not compare kubernetesVersion with apiserver. Only the kubernetesVersion in the operator.yaml is verified.

@harryge00
Copy link
Contributor Author

harryge00 commented Jan 21, 2020

golangci-lint run
pkg/kudoctl/cmd/verify/verify.go:5: File is not `goimports`-ed with -local github.com/kudobuilder (goimports)
	"github.com/kudobuilder/kudo/pkg/version"
make: *** [Makefile:48: lint] Error 1

make lint failed. Isn't github.com/kudobuilder/kudo/pkg/version a local package?

@gerred
Copy link
Member

gerred commented Jan 21, 2020

What's the diff when goimports is run on the file? Is there any? CircleCI lint is passing, so not sure what's happening locally that's different.

Copy link
Member

@nfnt nfnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition, we should make sure that kubernetesVersion is validated if set.
Though, currently kubernetesVersion is an optional field. I.e., we need to support cases where kubernetesVersion == "", which is also valid. Please support the case where kubernetesVersion isn't set.

So the test where kubernetesVersion == "" should result in a successful validation while something like kubernetesVersion == "invalid" should fail the validation.

@harryge00
Copy link
Contributor Author

harryge00 commented Jan 23, 2020

@nfnt When kubernetesVersion is not set, kubectl kudo install will fail with Error: unable to parse operators kubernetes version: Invalid Semantic Version. If kubernetesVersion is an optional field, should we also change the behavior of kudo install?

porridge@beczulka:~/tmp/o$ PATH=/home/porridge/Desktop/work/mesosphere/code/kudo/bin:$PATH kubectl kudo install .
Error: unable to parse operators kubernetes version: Invalid Semantic Version
porridge@beczulka:~/tmp/o$ PATH=/home/porridge/Desktop/work/mesosphere/code/kudo/bin:$PATH kubectl kudo package verify .
package is valid
porridge@beczulka:~/tmp/o$ cat operator.yaml 
apiVersion: kudo.dev/v1beta1
name: "minimal"
version: "0.1.0"
tasks:
- name: app
  kind: Apply
  spec:
    resources:
    - namespace.yaml
plans:
  deploy:
    strategy: serial
    phases:
    - name: main
      strategy: parallel
      steps:
      - name: app
        tasks:
        - app

@nfnt
Copy link
Member

nfnt commented Jan 23, 2020

Interesting! I wasn't aware of this behavior in kubectl kudo install. Only looked at the type. Then we should keep the behavior already enforced by kubectl kudo install. No changes needed from you.

@nfnt nfnt merged commit 34a93d7 into kudobuilder:master Jan 23, 2020
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

package verify does not check kubernetes version
3 participants