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.

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

Automatically update quickstart to use the latest image #1354

Closed
janewang opened this issue Jun 3, 2024 · 11 comments · Fixed by #1546
Closed

Automatically update quickstart to use the latest image #1354

janewang opened this issue Jun 3, 2024 · 11 comments · Fixed by #1546
Assignees
Labels
cli Related to Soroban CLI

Comments

@janewang
Copy link
Contributor

janewang commented Jun 3, 2024

This came up from a discussion topics on quickstart on May 28

@janewang janewang changed the title Have a command in network to update the quickstart image to use the latest Have a command in network to update the quickstart to use the latest image Jun 3, 2024
@janewang janewang added the cli Related to Soroban CLI label Jun 3, 2024
@willemneal
Copy link
Member

I would add that we should have an automated way of pinning the image tag used in the CLI at compile time. Currently it uses latest when using local and pubnet, and the respective testing/future . So technically this issue is the already solved.

Also currently there is an option to pass an image tag.

@leighmcculloch
Copy link
Member

leighmcculloch commented Jun 6, 2024

we should have an automated way of pinning the image tag used in the CLI at compile time

What problem are we solving by pinning the image version? There are competing concerns here. On the one hand stability with the CLI. On the other, when connecting to networks it is helpful to stay up-to-date with minor releases. The quickstart image very rarely breaks it's own UI because it has no major version to increment, so the quickstart image already aims to provide stability.

@leighmcculloch
Copy link
Member

leighmcculloch commented Jun 6, 2024

Have a command in network to update the quickstart to use the latest image

Rather than introduce a command I think we could change the CLI to attempt to update the image when connecting to testnet or pubnet, although we should make it tolerable to no connectivity, especially when running the local image.

Taking the approach of updating automatically makes for one less problem the user has to solve.

@janewang janewang changed the title Have a command in network to update the quickstart to use the latest image Automatically update quickstart to use the latest image Aug 6, 2024
@elizabethengelman
Copy link
Contributor

I want to make sure I understand where we landed on this issue. When starting up a network container with the CLI we could check to see if a newer version of the default image exists, and if so try to pull it. If there is no network connectivity, then use whatever image the user already has and notify them that there is a newer version available. Is that the idea?

Also, for reference, these are the current defaults for which images are being used for each network:
- pubnet => latest
- futurenet => future
- local => testing
- testnet => testing

@elizabethengelman
Copy link
Contributor

As I look at this a bit closer, I wonder if it would make sense to include an additional flag available to stellar network container start like --pull (or something?) that would force a pull from docker hub, otherwise it would use the local cached image for the given tag if possible.

@janewang
Copy link
Contributor Author

stellar network container start --pull-latest or stellar network container start --latest?

@elizabethengelman
Copy link
Contributor

I'm a bit hesitant to use the word latest in the flag name because to me that would imply that it is using the latest tag... when we are really trying to get the most up-to-date version of whatever tag that is correlated to network the user passes in.

@leighmcculloch
Copy link
Member

--pull is consistent with docker build and docker run tooling and sounds good to add with that name. Note that on the docker run command the --pull option has a value of always, missing, never and I think we should follow suit.

I think it's worth noting that it's also awkward option to use in an everyday flow, so the --pull always option on its own isn't really enough to deal with "automatic" feature request of this.

An average dev probably won't always use --pull always because they don't need to always pull.

However a dev probably won't remember to on some cadence change the command they're using to include --pull and will accidentally use an old version. So in our case I don't think --pull missing is the best default, even though it is the default for docker run.

If we specify the --pull as having options always, daily, missing, never, where daily is default, we can largely rely on the functionality of the docker run --pull <...> command, and when it is set to daily check the datetime the image was last pulled (which we can get from docker) and compare it to current.

To determine if the image has been pulled today, we can use the output of the docker inspect command to get the last time that it was pulled. I assume the docker lib we're using should have a way to get at the inspect output.

$ docker inspect stellar/quickstart:testing | jq -r '.[].Metadata.LastTagTime'
2024-08-13T21:27:04.970543219Z

@elizabethengelman
Copy link
Contributor

At the moment, we are doing the docker pull separately from docker run every time the network container start is called, as that is how bollard expects us to handle it:

  • docker.create_image - calls docker pull; if the local image differs from what is on dockerhub it will do the pull. if the local image has the same digest as what is on docker hub, it will not update because it's already up-to-date.
  • docker.create_container & docker.start_container - calls docker run

So, as it is, we are already updating the image to the most recent of that tag whenever we're running start.

From what I've seen, it doesn't look like bollard has an option that allows for passing in the --pull flag on docker run. If we do want the functionality that allows a user to use their local image, and not update when they want to... we could do something like this:

let container_response = docker.create_container

if container_response has an error like this "No such image: stellar/quickstart:testing" then we know its because the image isn't on the user's machine yet, and we can then call `docker.create_image` to pull it.

This would mean that the the --pull missing option is built into the start behaviour. And we could find ways to replicate the other --pull options.

But, I'm not sure if that is worth the effort right now. @janewang @leighmcculloch what do you think?

Either way, I am going to add some more logging in here, so that we can tell the user if/when we are indeed pulling the newer image, and to inform them of the image digest that is being used.

@janewang
Copy link
Contributor Author

I think a bit logging notifying user of this behavior of pulling the latest suffices.

@leighmcculloch
Copy link
Member

+1 a bit of logging, plus it would be worth handling the case we can't pull and continuing with using what's available locally. And we can do all of that without any new options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to Soroban CLI
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants