30x / shipyardctl

command line interface for Shipyard API
Apache License 2.0
0 stars 3 forks source link

Refactor command names, arguments and options #51

Closed whitlockjc closed 7 years ago

whitlockjc commented 7 years ago

We recently discussed shipyardctl and how it maps not only to Apigee but also within the ecosystem of other Apigee CLIs. Here is the latest design document: https://docs.google.com/document/d/1yZSSrH1kIwxT5VNwkB6HQxTpY90VjwLAlcPj7P42xj8/edit Based on this, we've decided to make a few changes:

Here is an example of the old shipyardctl create image command for the new format:

shipyardctl import application --name "echo-app1[:1]" --path "9000:/echo-app" \
--directory . --org acme --runtime node:4 --env-var {name1}={value1} --env-var {name2}={value2}
whitlockjc commented 7 years ago

Since it might not be clear, I've updated the --name and --revision options above so before implementing, please read them again. Long story short, they are being combined to follow more of a Docker-style approach.

whitlockjc commented 7 years ago

Also, --env is no longer used for environment variables, --env-var is.

whitlockjc commented 7 years ago

As you implement this, let me know if --runtime is bothersome. As it stands now, the naming of it is forward thinking but since Shipyard only supports node.js, having to specify node: when overriding the Node.js version could appear superfluous. I'm just not sure of a better way.

noahdietz commented 7 years ago

@whitlockjc when deploying an application, are we still requiring publicHosts & privateHosts?

If I remember correctly, the accepted hosts are specified at the environment level and retrieved from Edge? So these arguments are no longer necessary.

noahdietz commented 7 years ago

Another question should deploy proxy make a proxy bundle on the fly, or only take an already created one, like via shipyardctl create bundle...? apigeetool only takes one via --directory, but I can make an in-memory copy and upload that if no --directory is given. What do you think?

whitlockjc commented 7 years ago

@whitlockjc when deploying an application, are we still requiring publicHosts & privateHosts?

If I remember correctly, the accepted hosts are specified at the environment level and retrieved from Edge? So these arguments are no longer necessary.

We are in the process of creating a new router. Until the new router is created, we should keep the public/private hosts stuff. I will create the necessary shipyardctl issues when that work is complete.

whitlockjc commented 7 years ago

Another question should deploy proxy make a proxy bundle on the fly, or only take an already created one, like via shipyardctl create bundle...? apigeetool only takes one via --directory, but I can make an in-memory copy and upload that if no --directory is given. What do you think?

We could make it handle both? If you don't specify the --directory option, we generate one for you based on sane defaults. If you do specify the --directory option, we assume you have an on-disk bundle and we'll deploy that.

Thoughts?

/cc @mpnally

noahdietz commented 7 years ago

we should keep the public/private hosts stuff

Ok, I'll make them required options --public-hosts & --private-hosts

noahdietz commented 7 years ago

We could make it handle both?

totally possible, let's do it

we generate one for you based on sane defaults

Does this get saved to disk in cwd? Or do we need to expose a --save option as well? I feel like the latter could get confusing...

whitlockjc commented 7 years ago

Actually, let's skip the hosts support for now. enrober will drive this.

mpnally commented 7 years ago

I don't think that Kiln should know anything about hostnames. Hostnames vary with deployment location, so are only an enrober concern

The version of hostnames I like for enrober goes like this:

Obviously, the current K8S "deployment" feature is not currently able to do this — we would be doing our own replacement for K8S deployments, whose function currently is not very useful.

Martin

On Thu, Dec 8, 2016 at 10:39 AM, Jeremy Whitlock notifications@github.com wrote:

Actually, let's skip the hosts support for now. enrober will drive this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/30x/shipyardctl/issues/51#issuecomment-265818601, or mute the thread https://github.com/notifications/unsubscribe-auth/ACoA6lKb8hFEuH9zTRzuuanWpupqfgzAks5rGE7jgaJpZM4LE1vQ .

jbowen93 commented 7 years ago

I'm seeing two key takeaways that need to be implemented for Martin's comment.

  1. Deployments can only serve traffic to hosts that are a subset of the environment hosts.

    Environment Hosts: [prod, debug, test]
    Deployment 1 Hosts: [prod]
    Deployment 2 Hosts: [debug]
  2. We should be able to modify which hosts go to a specific revision of a deployment.

    Deployment 1 v2 Hosts: [canary]
    Deployment 1 v1 Hosts: [prod]

I believe the first issue can be solved through enrober. When a deployment is made enrober will check to ensure that the requested hosts are one of the Edge hostAliases (I use this word because it is the accepted term for Edge).

For the second issue this can be accomplished using k8s deployments in a variety of ways. This is largely dependent on the router that we are using. I believe the simplest way to do this is to have all deployments created with enrober be versioned. You can just create a new deployment with the new version. When canary testing is done on the new version you can simply scale down the old version. Obviously this is more manual than desirable but I feel a full CI/CD pipeline that would automatically do this is outside of the scope of shipyard at this time.

mpnally commented 7 years ago

@jbowen93 I agree with what you wrote above. I agree also that we should defer implementing this for now. Controlling hosts for a deployment only has value if we have a larger design for how new revisions get introduced incrementally via a defined workflow. For now, I think we should remove the ability to specify hostnames for a deployment — the deployment will receive traffic for all the hosts listed at the environment (namespace) level. We can start a separate issue to work on a design for more controlled introduction of revisions.