No flags to support container port information while creating service

[savitaashture]

This is kind of enhancement to the existing flags of kn service command

Description:
When someone writes application which listens on port other than 8080(default port in knative-serving) then deploy using cli(kn) then there is no way to specify the port.

Current:
Supported flags are

Flags:
  -e, --env stringArray          Environment variable to set. NAME=value; you may provide this flag any number of times to set multiple environment variables.
      --force                    Create service forcefully, replaces existing service if any.
  -h, --help                     help for create
      --image string             Image to run.
      --limits-cpu string        The limits on the requested CPU (e.g., 1000m).
      --limits-memory string     The limits on the requested CPU (e.g., 1024Mi).
  -n, --namespace string         List the requested object(s) in given namespace.
      --requests-cpu string      The requested CPU (e.g., 250m).
      --requests-memory string   The requested CPU (e.g., 64Mi).

Expected:
Along with current flags it should support container-port flag so that user can specify port at which application is running.

ex: something like below

kn service create --container-port 80 

[savitaashture]

/assign

[sixolet]

This is not part of the knative contract. The knative system supplies the container the enviornment variable PORT, and the container must listen on that port.

[rhuss]

This is not part of the knative contract. The knative system supplies the container the enviornment variable PORT, and the container must listen on that port.

Ah, that's new to me. I thought that it's allowed to specify one single port int the container spec of the service / revision CRD.

@sixolet could you please point me to which knative contract do you refer to ?

[rhuss]

According to https://github.com/knative/serving/blob/master/docs/spec/spec.md the port can be specified in the CRD:

  # Only a single containerPort may be specified.
  # This can be specified to select a specific port for incoming traffic.
  # This is useful if your application cannot discover the port to listen
  # on through the $PORT environment variable that is always set within the container.
  # Some fields are not allowed, such as hostIP and hostPort.
  ports: # Optional
    # Valid range is [1-65535], except 8012 and 8013 (queue proxy request ports)
    # 8022 (queue proxy admin port), 9090 and 9091 (queue proxy metrics ports).
    - containerPort: ...
      name: ... # Optional, one of "http1", "h2c"
      protocol: ... # Optional, one of "", "tcp"

So I think a kn service create should be able to specify that, too ?

[evankanderson]

The Service spec allows the developer to specify the container.ports[0].containerPort argument, which should be reflected into the PORT environment variable when the container is started.

[rhuss]

The Service spec allows the developer to specify the container.ports[0].containerPort argument, which should be reflected into the PORT environment variable when the container is started.

Thanks for the clarification @evankanderson . @sixolet I think this issue is still a valid use case as it should be possible for the user to specify this single port on the command line like in kn service create --image myimage --port 9090 which should end up in container.ports[0].containerPort (or containers[0].ports[0].containerPort for >= 0.7.0) when actually creating the service.

[savitaashture]

Yes Service spec allows to specify port ..Because by default if no port is specified it will take 8080 as containerPort.

But if application is running on some other port then we have to provide a option to user to provide a port information when they use CLI(kn).

[sixolet]

... Damn, I guess the container runtime contract has changed since last I read it.

…

On May 28, 2019, at 9:29 PM, savitaashture ***@***.***> wrote: Yes Service spec allows to specify port ..Because by default if no port is specified it will take 8080 as containerPort. But if application is running on some other port then we have to provide a option to user to provide a port information when they use CLI(kn). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[savitaashture]

@sixolet

If we agreed to implement container port than can you reopen the issue...

[evankanderson]

I think it may have been misunderstood or mis-read. Customers may want to manage an existing container under Knative which wasn't written to depend on the $PORT parameter, so containerPort allows dictating to the infrastructure which port to deliver traffic on. On Wed, May 29, 2019 at 12:10 AM Naomi Seyfer <[email protected]> wrote:

…

... Damn, I guess the container runtime contract has changed since last I read it. > On May 28, 2019, at 9:29 PM, savitaashture ***@***.***> wrote: > > Yes Service spec allows to specify port ..Because by default if no port is specified it will take 8080 as containerPort. > > But if application is running on some other port then we have to provide a option to user to provide a port information when they use CLI(kn). > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#142?email_source=notifications&email_token=AB4XEN7AXAC7GP4AJMDXH4LPXYT5NA5CNFSM4HP4EVGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWOMOMI#issuecomment-496813873>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB4XENZPNI2KHE6JITB6PB3PXYT5NANCNFSM4HP4EVGA> .

-- Evan Anderson <[email protected]>

[duglin]

+1 this is very important for existing images out there that do not support $PORT or listening on port 8080.

One suggestion, can we name it --port instead of --container-port? Just to require less typing :-)

@savitaashture are you working on this?

[savitaashture]

@duglin Yes iam working on it :)

Completed code changes need to raise PR.

[savitaashture]

typing

+1
Yes it will be easy for typing :)

[navidshaikh]

Fixed by #191