Giter Club home page Giter Club logo

Comments (12)

maximilien avatar maximilien commented on July 24, 2024 1

OK, @rhuss I get your point. However, I would vote for one command and allowing multiple flags as needed. Nothing prevents having multiple --env ... on a request?

And if more env manipulations are needed then we can explore special command.

from client.

evankanderson avatar evankanderson commented on July 24, 2024 1

Not sure if this will apply here, but I have seen situations in the past where multiple changes needed to be done together, or at least in a specified sequence. To pick on an example that makes me sad every time:

kn service update kaffe --request-mem 1024Mb --env JAVA_XMX=700M

(assuming that the actual args to java -jar foo.jar includes an -Xmx${JAVA_XMX} flag)

Other items that can vary similarly:

  • ServiceAccount and Env Vars (for repointing to another database)
  • Concurrency / Timeout and args

from client.

csantanapr avatar csantanapr commented on July 24, 2024 1

What type of env vars was discussed in one of the WG meetings when defining the MVP for the first release and group agree to keep it simple and implement first only simple strings for env vars

from client.

csantanapr avatar csantanapr commented on July 24, 2024

/milestone v0.1.0

from client.

knative-prow-robot avatar knative-prow-robot commented on July 24, 2024

@csantanapr: You must be a member of the knative/knative-milestone-maintainers github team to set the milestone.

In response to this:

/milestone v0.1.0

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.

from client.

maximilien avatar maximilien commented on July 24, 2024

OK, so maybe this one can be my next victim :)

@sixolet are you thinking update for all params but the image, so for instance:

kn service update old-name --name new-name --request-cpu 1000m --limits-memory 1024Mi

This would update the service's name to new-name and any values in the request CPU and limits memory as specified.

kn service update new-name --env NEW_KEY=NEW_VAL KEY=NEW_VAL --limits-cpu 1000m

This would update the service with new-name with a new environment variable NEW_KEY and update the old environment variable KEY with NEW_VAL

And so on.

from client.

rhuss avatar rhuss commented on July 24, 2024

name is immutable, so for an update, you would have to delete and create the service. This is true for Kubernetes resources in general.

Do we have (or is it planned) to have a rollout or deploy command for updating the image ? If so, I agree to no allow to change the image here. Otherwise changing the image via an update is an easy way to trigger a redeployment.

For an analogy to kubectl: kubectl uses kubectl set image or kubectl set env etc. to change specific fields.

I'm not sure what would be nicer in our context:

kn service set env ....  # all possible options that 'kubectl set env' supports
kn service set image ...
kn service set ...

or

kn service update --image=... --env ....

I think the former is more straight forward if you want to support multiple ways for e.g. updating env vars (like from a file, or managing multiple env, or other stuff). Interestingly there in kubectl set env there are also very ugly things like kubectl set env pods --all --list for only listing env without changing them. We shouldn't copy that here :)

from client.

maximilien avatar maximilien commented on July 24, 2024

I like kn service update --image=... --env .... reads better (English) and results in less commands.

But happy to do whichever. At this point just want to move forward.

cc: @sixolet and @cppforlife what say you? I want to get started on this today or tomorrow. Please provide your input please. No input tomorrow then I start with @rhuss suggestion. We can always change later if a strong opinion emerges.

from client.

rhuss avatar rhuss commented on July 24, 2024

@maximilien it really depends how much power you want to add for changing e.g. env.

If it's ok to update single env vars only (potentially with the option to remove/add one, too, then I agree doing it per options makes is more lighweight.

If we want to support all env var manipulating features of kubectl like:

  • Specifying env literally
  • Reading env vars from a file
  • Using a configmap / secrret for the env var

then a dedicated subcommand could make more sense to not overwhelm the number of options required.

from client.

cppforlife avatar cppforlife commented on July 24, 2024

my vote is for having additional flags on update command.

from client.

maximilien avatar maximilien commented on July 24, 2024

OK I'll move forward with kn service update --image=... --env .... and monitor here for additional opinions or comments. Cheers 🍻

from client.

rhuss avatar rhuss commented on July 24, 2024

Not sure if this will apply here, but I have seen situations in the past where multiple changes needed to be done together, or at least in a specified sequence.

If you require the changes applied in a specific sequence, using options means to imply an order on the options given. Not sure if you can easily find that out with the current libraries we are using for option handling (and its a bad UX anyway, as no one else imposes semantics on an option ordering. Arguments would be used for that).

I agreed that its good for situations where you want to update multiples aspects atomically.

So I'm fine with options as long as we don't impose an option order but apply all changes with a single update call.

wrt/ to env: We should restrict ourselves here to support literal env vars (no configmap/secret refs) and support multiple options given with --env to update multiple envs at once. Also supporting the syntax --env TOREMOVENV- to remove envs (i.e. a trailing - indicates the removal of an env, like kubectl set env does) would be awesome.

from client.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.