Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Upgrade Kubebuilder to v1.0.3. #250

Merged
merged 1 commit into from
Sep 11, 2018

Conversation

gyliu513
Copy link
Contributor

@gyliu513 gyliu513 commented Sep 6, 2018

Kubebuider v1.0.3 is shipping with Kubernetes 1.11, there is no need
to download the v1.11 kube-apiserver in test script, and also no need
to update hack/install.yaml, as the issue kubernetes/kubernetes#65293
was already fixed in kubernetes v1.11.

/cc @marun @pmorie

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 6, 2018
@gyliu513 gyliu513 force-pushed the kubebuilder-1.0.3 branch 3 times, most recently from f3910a4 to eb3e60f Compare September 6, 2018 09:22
@marun
Copy link
Contributor

marun commented Sep 6, 2018

Thank you for doing this. Please drop the changes to install-latest.yaml. It needs to be changed in concert with the :latest tagged image.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 6, 2018
@gyliu513
Copy link
Contributor Author

gyliu513 commented Sep 6, 2018

@marun done

kubernetes 1.10. Once `kubebuilder` bundles binaries >= 1.11, it will
only require downloading the `kubebuilder` release.
([v1.0.3](https://github.com/kubernetes-sigs/kubebuilder/releases/tag/v1.0.3)
as of this writing) release packages all of these dependencies together and
Copy link
Contributor

Choose a reason for hiding this comment

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

Please end the sentence at 'together' - I think the remainder is redundant.

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

@@ -323,18 +323,6 @@ sed -i 's/memory: 20Mi/memory: 64Mi/' hack/install.yaml
sed -i 's/memory: 30Mi/memory: 128Mi/' hack/install.yaml
```

Then, delete the `type` field from the OpenAPI schema to avoid triggering validation
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this fix from scripts/generate-install-yaml.sh as well.

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

@marun
Copy link
Contributor

marun commented Sep 7, 2018

Please squash - we aren't using a fixup workflow.

@gyliu513
Copy link
Contributor Author

@marun using one commit now.

@shashidharatd
Copy link
Contributor

The changes LGTM. Thanks @gyliu513, We don't have to use the workaround to manually fix the generated code from now on :)
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2018
Kubebuider v1.0.3 is shipping with Kubernetes 1.11, there is no need
to download the v1.11 kube-apiserver in test script, and also no need
to update `hack/install.yaml`, as the issue kubernetes/kubernetes#65293
was already fixed in kubernetes v1.11.
@k8s-ci-robot k8s-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2018
@marun
Copy link
Contributor

marun commented Sep 10, 2018

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 10, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gyliu513, marun

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

The pull request process is described 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 10, 2018
@gyliu513
Copy link
Contributor Author

I restart CI and it passed, but the report still failed. Do you know what is wrong? @marun

@k8s-ci-robot k8s-ci-robot merged commit 054b998 into kubernetes-retired:master Sep 11, 2018
@gyliu513 gyliu513 deleted the kubebuilder-1.0.3 branch September 11, 2018 02:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants