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

Update to Kubernetes v1.18.2 #207

Merged
merged 3 commits into from Oct 16, 2020
Merged

Update to Kubernetes v1.18.2 #207

merged 3 commits into from Oct 16, 2020

Conversation

nan-yu
Copy link
Contributor

@nan-yu nan-yu commented Oct 9, 2020

No description provided.

tamalsaha and others added 2 commits September 24, 2020 13:49
Signed-off-by: Tamal Saha <tamal@appscode.com>
- merge app.k8s.io_applications.yaml with app.k8s.io_applications.v1.yaml
to get rid of the v1beta1 version of CRD
- replace the deprecated apiextensions.k8s.io/v1beta1 version with
apiextensions.k8s.io/v1beta1
- add `kubectl` to the PATH as the latest controller-gen depends on
`kubectl`
- remove `kind get kubeconfig-path --name="kind"` as `kubeconfig-path`
was depracated in kind v0.8.1
- delete an empty file
- update customerreource.go to use the v1 API instead of v1beta1
- add license to the generated resources
- remove the extra `type` field from the test application.yaml file
- add the `version` field to the test GVK
@nan-yu
Copy link
Contributor Author

nan-yu commented Oct 9, 2020

This PR was built on top of Tamal's PR: #199 with some fixes.

Thank you, @tamalsaha for updating Kubernetes's version.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 9, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nan-yu
To complete the pull request process, please assign kow3ns after the PR has been reviewed.
You can assign the PR to them by writing /assign @kow3ns in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 9, 2020
@@ -44,7 +44,7 @@ var _ = Describe("Application Reconciler", func() {
var labelSet1 = map[string]string{"foo": "bar"}
var labelSet2 = map[string]string{"baz": "qux"}
var namespace1 = metav1.NamespaceDefault
var namespace2 = "default2"
var namespace2 = "kube-system"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Not if the tests pass. I could not find where this "default2" namespace was created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With controller-runtime upgraded from v0.4.0 to v0.6.0, the test failed with namespace not found error. I explicitly created the namespace in the test to avoid using the kube-system namespace.

Makefile Outdated
$(TOOLBIN)/controller-gen:
GOBIN=$(TOOLBIN) GO111MODULE=on go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.5
$(TOOLBIN)/controller-gen: $(TOOLBIN)/kubectl
GOBIN=$(TOOLBIN) GO111MODULE=on go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.3.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be updated to v0.4.0? When I originally created this commit, v0.4.0 was not out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

controller-gen@v0.4.0 seems to be incompatible with client-go@v0.18.2. I got the following error when updating to v0.4.0:

17:14 $ make manifests 
/usr/local/google/home/nanyu/go/src/sigs.k8s.io/application/hack/tools/bin/controller-gen \
        "crd:trivialVersions=true,crdVersions=v1" \
        rbac:roleName=kube-app-manager-role \
        paths=./... \
        output:crd:artifacts:config=config/crd/bases \
        output:crd:dir=config/crd/bases \
        output:webhook:dir=config/webhook \
        webhook
../../../../pkg/mod/k8s.io/client-go@v0.18.2/discovery/discovery_client.go:30:2: module github.com/googleapis/gnostic@latest found (v0.5.1), but does not contain package github.com/googleapis/gnostic/OpenAPIv2
../../../../pkg/mod/k8s.io/kube-openapi@v0.0.0-20200805222855-6aeccd4b50c6/pkg/util/proto/document.go:24:2: case-insensitive import collision: "github.com/googleapis/gnostic/openapiv2" and "github.com/googleapis/gnostic/OpenAPIv2"
Error: not all generators ran successfully
run `controller-gen crd:trivialVersions=true,crdVersions=v1 rbac:roleName=kube-app-manager-role paths=./... output:crd:artifacts:config=config/crd/bases output:crd:dir=config/crd/bases output:webhook:dir=config/webhook webhook -w` to see all available markers, or `controller-gen crd:trivialVersions=true,crdVersions=v1 rbac:roleName=kube-app-manager-role paths=./... output:crd:artifacts:config=config/crd/bases output:crd:dir=config/crd/bases output:webhook:dir=config/webhook webhook -h` for usage

I would keep using v0.3.0 with K8s v1.18.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refreshed all the dependencies and it worked now with controller-gen v0.4.0.

Makefile Outdated
@@ -59,16 +59,17 @@ $(TOOLBIN)/mockgen:
GOBIN=$(TOOLBIN) GO111MODULE=on go get github.com/golang/mock/mockgen@v1.3.1

$(TOOLBIN)/conversion-gen:
GOBIN=$(TOOLBIN) GO111MODULE=on go get k8s.io/code-generator/cmd/conversion-gen@v0.17.0
GOBIN=$(TOOLBIN) GO111MODULE=on go get k8s.io/code-generator/cmd/conversion-gen@v0.18.2
Copy link
Contributor

Choose a reason for hiding this comment

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

May be use the latest v0.18.9 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DONE.

@@ -8,7 +8,7 @@ VERSION_FILE ?= VERSION-DEV
include $(VERSION_FILE)

# Produce CRDs that work back to Kubernetes 1.11 (no version conversion)
CRD_OPTIONS ?= "crd:trivialVersions=true"
CRD_OPTIONS ?= "crd:trivialVersions=true,crdVersions=v1"
Copy link
Contributor

Choose a reason for hiding this comment

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

How far behind, the generated CRDs should be supported? crdVersions v1beta1 is still needed for kubernetes < 1.16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's up to the user to generate the v1beta1 version of the application CRD.
I added a comment to show how to set the CRD_OPTIONS for the v1beta1 version.

Copy link
Contributor Author

@nan-yu nan-yu left a comment

Choose a reason for hiding this comment

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

Travis build passed with the latest changes. Please re-review this PR.

@tamalsaha
Copy link
Contributor

FWIW, lgtm . Thanks, @nan-yu !

@nan-yu nan-yu merged commit c8e2959 into kubernetes-sigs:master Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants