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.

Sign up for GitHub

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

Jump to bottom

explicitly specify port protocol field to allow server side apply #4050

Merged
merged 1 commit into from
Aug 4, 2021
Merged

explicitly specify port protocol field to allow server side apply #4050

merged 1 commit into from
Aug 4, 2021

Conversation

longkai
Copy link
Contributor

@longkai longkai commented May 22, 2021 •
edited

What this PR does / why we need it:

Actually it's a k8s bug.

In order to allow server side apply in most running clusters currently, we have to explicitly specify port protocol field in yaml manifest.

Which issue this PR fixes: fixes #4020

Special notes for your reviewer:

This is the result after specifying the protocol field in the manifest:

$ k apply --server-side -f cert-manager.yaml
customresourcedefinition.apiextensions.k8s.io/certificaterequests.cert-manager.io serverside-applied
customresourcedefinition.apiextensions.k8s.io/certificates.cert-manager.io serverside-applied
customresourcedefinition.apiextensions.k8s.io/challenges.acme.cert-manager.io serverside-applied
customresourcedefinition.apiextensions.k8s.io/clusterissuers.cert-manager.io serverside-applied
customresourcedefinition.apiextensions.k8s.io/issuers.cert-manager.io serverside-applied
customresourcedefinition.apiextensions.k8s.io/orders.acme.cert-manager.io serverside-applied
namespace/cert-manager serverside-applied
serviceaccount/cert-manager-cainjector serverside-applied
serviceaccount/cert-manager serverside-applied
serviceaccount/cert-manager-webhook serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-cainjector serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-issuers serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-clusterissuers serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-certificates serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-orders serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-challenges serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-ingress-shim serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-view serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-edit serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-controller-approve:cert-manager-io serverside-applied
clusterrole.rbac.authorization.k8s.io/cert-manager-webhook:subjectaccessreviews serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-cainjector serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-issuers serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-clusterissuers serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-certificates serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-orders serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-challenges serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-ingress-shim serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-controller-approve:cert-manager-io serverside-applied
clusterrolebinding.rbac.authorization.k8s.io/cert-manager-webhook:subjectaccessreviews serverside-applied
role.rbac.authorization.k8s.io/cert-manager-cainjector:leaderelection serverside-applied
role.rbac.authorization.k8s.io/cert-manager:leaderelection serverside-applied
role.rbac.authorization.k8s.io/cert-manager-webhook:dynamic-serving serverside-applied
rolebinding.rbac.authorization.k8s.io/cert-manager-cainjector:leaderelection serverside-applied
rolebinding.rbac.authorization.k8s.io/cert-manager:leaderelection serverside-applied
rolebinding.rbac.authorization.k8s.io/cert-manager-webhook:dynamic-serving serverside-applied
service/cert-manager serverside-applied
service/cert-manager-webhook serverside-applied
deployment.apps/cert-manager-cainjector serverside-applied
deployment.apps/cert-manager serverside-applied
deployment.apps/cert-manager-webhook serverside-applied
mutatingwebhookconfiguration.admissionregistration.k8s.io/cert-manager-webhook serverside-applied
validatingwebhookconfiguration.admissionregistration.k8s.io/cert-manager-webhook serverside-applied
NONE

@jetstack-bot jetstack-bot added dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. area/deploy Indicates a PR modifies deployment configuration needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 22, 2021
@jetstack-bot
Copy link
Contributor

Hi @longkai. Thanks for your PR.

I'm waiting for a jetstack or cert-manager member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@jetstack-bot jetstack-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 22, 2021
@longkai
fix(ssa): explicitly specify port protocol field
1e95a8e 
Actually it's a [k8s bug](kubernetes/kubernetes@dab5112).

In order to allow server side apply in most running clusters currently, we have to explicitly specify port `protocol` field in yaml manifest.

Signed-off-by: longkai <im.longkai@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. and removed dco-signoff: no Indicates that at least one commit in this pull request is missing the DCO sign-off message. labels May 22, 2021
@longkai
Copy link
Contributor Author

longkai commented May 22, 2021

/assign @munnerz

@longkai
Copy link
Contributor Author

longkai commented Jun 28, 2021

@munnerz hello, is there any updates?

@jakexks
Copy link
Member

jakexks commented Aug 4, 2021

/ok-to-test
/kind bug
/approve
/lgtm

@jetstack-bot jetstack-bot added kind/bug Categorizes issue or PR as related to a bug. ok-to-test lgtm Indicates that a PR is ready to be merged. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Aug 4, 2021
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jakexks, longkai

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

@jetstack-bot jetstack-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2021
@jetstack-bot jetstack-bot merged commit 34cb511 into cert-manager:master Aug 4, 2021
@jetstack-bot jetstack-bot added this to the v1.5 milestone Aug 4, 2021
@maelvls maelvls mentioned this pull request Mar 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@munnerz munnerz

@jakexks jakexks

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/deploy Indicates a PR modifies deployment configuration dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. ok-to-test release-note-none Denotes a PR that doesn't merit a release note. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

server side apply failed
4 participants
@longkai @jetstack-bot @jakexks @munnerz