Re: [kubernetes/kubernetes] kubectl create ingress command (#44534)
Fabiano Franz
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
Fabiano Franz
@fabianofranz commented on this pull request.
In pkg/kubectl/cmd/create_ingress.go:
> +func NewCmdCreateIngress(f cmdutil.Factory, cmdOut io.Writer) *cobra.Command {
+ cmd := &cobra.Command{
+ Use: "ingress NAME --host=host.example.com [--tls-acme] [--dry-run]",
+ Short: i18n.T("Create an Ingress with the specified name."),
+ Long: ingressLong,
+ Example: ingressExample,
+ Run: func(cmd *cobra.Command, args []string) {
+ err := CreateIngress(f, cmdOut, cmd, args)
+ cmdutil.CheckErr(err)
+ },
+ }
+ cmdutil.AddApplyAnnotationFlags(cmd)
+ cmdutil.AddValidateFlags(cmd)
+ cmdutil.AddPrinterFlags(cmd)
+ cmdutil.AddGeneratorFlags(cmd, cmdutil.IngressV1Beta1GeneratorName)
+ cmd.Flags().StringSlice("host", []string{}, "Host name for the ingress record")
Looks like this flag is required, so it needs cmd.MarkFlagRequired.
Fabiano Franz
> + cmd := &cobra.Command{
+ Use: "ingress NAME --host=host.example.com [--tls-acme] [--dry-run]",
+ Short: i18n.T("Create an Ingress with the specified name."),
+ Long: ingressLong,
+ Example: ingressExample,
+ Run: func(cmd *cobra.Command, args []string) {
+ err := CreateIngress(f, cmdOut, cmd, args)
+ cmdutil.CheckErr(err)
+ },
+ }
+ cmdutil.AddApplyAnnotationFlags(cmd)
+ cmdutil.AddValidateFlags(cmd)
+ cmdutil.AddPrinterFlags(cmd)
+ cmdutil.AddGeneratorFlags(cmd, cmdutil.IngressV1Beta1GeneratorName)
+ cmd.Flags().StringSlice("host", []string{}, "Host name for the ingress record")
+ cmd.Flags().Bool("tls-acme", false, "Enables ACME (LetEncrypt) support for automatic TLS")
Let’s Encrypt
Fabiano Franz
@justinsb Sorry about the delay! Just a couple minor comments, make sure you squash then this is good to go.
Justin Santa Barbara
Thanks @fabianofranz - should be all fixed up & squashed
Fabiano Franz
> + + "github.com/spf13/cobra" + + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/kubernetes/pkg/kubectl" + "k8s.io/kubernetes/pkg/kubectl/cmd/templates" + cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util" + "k8s.io/kubernetes/pkg/util/i18n" +) + +var ( + ingressLong = templates.LongDesc(i18n.T(` + Create an ingress with the specified name.`)) + + ingressExample = templates.Examples(i18n.T(` + # Create a new ingress named my-ingress supporting TLS with LetsEncrypt
Let’s Encrypt
Fabiano Franz
should be all fixed up & squashed
not yet, forgot to git push? ;)
k8s-ci-robot
@justinsb: The following test(s) failed:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| Jenkins Bazel Build | 27067d1 | link | @k8s-bot bazel test this |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
Justin Santa Barbara
Justin Santa Barbara
Thanks @fabianofranz - I had indeed failed to push! Should be fixed now!
Fabiano Franz
LGTM from a CLI perspective, @smarterclayton mind taking a conceptual look at this?
Justin Santa Barbara
/assign @smarterclayton
Justin Santa Barbara
ping @smarterclayton
Kubernetes Submit Queue
@justinsb PR needs rebase
k8s-ci-robot
@justinsb: The following test(s) failed:
| Test name | Commit | Details | Rerun command |
|---|
| pull-kubernetes-federation-e2e-gce | 6e5966a | link | @k8s-bot pull-kubernetes-federation-e2e-gce test this |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
—
k8s-ci-robot
k8s-ci-robot
@justinsb: The following test(s) failed:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| pull-kubernetes-federation-e2e-gce | 6e5966a | link | @k8s-bot pull-kubernetes-federation-e2e-gce test this |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
—
Justin Santa Barbara
@k8s-bot pull-kubernetes-federation-e2e-gce test this
grodrigues3
@justinsb this looks like a feature that was not lgtm'd prior to code freeze. Removing from the milestone.
Justin Santa Barbara
ping @smarterclayton
k8s-ci-robot
@justinsb: The following test failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|
| pull-kubernetes-bazel | 9cea83f | link | @k8s-bot pull-kubernetes-bazel test this |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
—
Peter Zhao
@xiangpengzhao commented on this pull request.
pull-kubernetes-bazel failed.
In pkg/kubectl/cmd/create_ingress_test.go:
> + t.CachedIngress = obj.(*extensions.Ingress)
+ return nil
+}
+
+func (t *testIngressPrinter) AfterPrint(output io.Writer, res string) error {
+ return nil
+}
+
+func (t *testIngressPrinter) HandledResources() []string {
+ return []string{}
+}
+
+func TestCreateIngress(t *testing.T) {
+ f, tf, _, _ := cmdtesting.NewAPIFactory()
+ printer := &testIngressPrinter{}
+ tf.Printer = printer
I0613 06:12:09.973] pkg/kubectl/cmd/create_ingress_test.go:53: cannot use printer (type *testIngressPrinter) as type printers.ResourcePrinter in assignment:
I0613 06:12:09.974] *testIngressPrinter does not implement printers.ResourcePrinter (missing IsGeneric method)
k8s-ci-robot
@justinsb: The following tests failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|
| pull-kubernetes-unit | 9cea83f | link | @k8s-bot pull-kubernetes-unit test this |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
—
Justin Santa Barbara
Justin Santa Barbara
/unassign ghodds
/unassign ericchiang
Justin Santa Barbara
/unassign ghodss
Nathan L Smith
I would like to shadow this code review
Kubernetes Submit Queue
@justinsb PR needs rebase
Kubernetes Submit Queue
This PR hasn't been active in 30 days. It will be closed in 59 days (Oct 11, 2017).
You can add 'keep-open' label to prevent this from happening, or add a comment to keep it open another 90 days
Kubernetes Submit Queue
This PR hasn't been active in 60 days. It will be closed in 29 days (Oct 11, 2017).
Kubernetes Submit Queue
This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review.
You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days
Kubernetes Submit Queue
Closed #44534.
k8s-ci-robot
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: justinsb
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: smarterclayton
If they are not already assigned, you can assign the PR to them by writing /assign @smarterclayton in a comment when ready.
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
Justin Santa Barbara
Reopened #44534.
k8s-ci-robot
@justinsb: The following tests failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|
| pull-kubernetes-bazel-build | 0b3038c | link | /test pull-kubernetes-bazel-build |
| pull-kubernetes-bazel-test | 0b3038c | link | /test pull-kubernetes-bazel-test |
| pull-kubernetes-integration | 0b3038c | link | /test pull-kubernetes-integration |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
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. I understand the commands that are listed here.
—
k8s-ci-robot
@justinsb: The following tests failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| pull-kubernetes-bazel-build | 0b3038c | link | /test pull-kubernetes-bazel-build |
| pull-kubernetes-bazel-test | 0b3038c | link | /test pull-kubernetes-bazel-test |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
k8s-ci-robot
@justinsb: The following test failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|---|---|---|
| pull-kubernetes-bazel-build | 0b3038c | link | /test pull-kubernetes-bazel-build |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
k8s-ci-robot
@justinsb: The following tests failed, say /retest to rerun them all:
| Test name | Commit | Details | Rerun command |
|---|
| pull-kubernetes-bazel-build | 0b3038c | link | /test pull-kubernetes-bazel-build |
| pull-kubernetes-bazel-test | 0b3038c | link | /test pull-kubernetes-bazel-test |
| pull-kubernetes-integration | 0b3038c | link | /test pull-kubernetes-integration |
| pull-kubernetes-e2e-kops-aws | 0b3038c | link | /test pull-kubernetes-e2e-kops-aws |
| pull-kubernetes-e2e-gce | 0b3038c | link | /test pull-kubernetes-e2e-gce |
| pull-kubernetes-e2e-gce-device-plugin-gpu | 0b3038c | link | /test pull-kubernetes-e2e-gce-device-plugin-gpu |
| pull-kubernetes-node-e2e | 0b3038c | link | /test pull-kubernetes-node-e2e |
k8s-ci-robot
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
k8s-ci-robot
| pull-kubernetes-e2e-gce-device-plugin-gpu | 0b3038c | link | /test pull-kubernetes-e2e-gce-device-plugin-gpu |
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
k8s-ci-robot
k8s-ci-robot
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
k8s-ci-robot
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.
fejta-bot
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale
fejta-bot
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten
Jordan Liggitt
given the in progress move to networking.k8s.io, and the impending removal of extensions/v1beta1, let's avoid adding new uses of extensions/v1beta1, especially in clients like kubectl that have longer skew support
Jordan Liggitt
closing this for now, can revisit once ingress v1 plans are completed in 1.15
/closew
Jordan Liggitt
Closed #44534.