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

air gapped installation: install option like --registry for prometheus image #3204

Closed
kellermaennchen opened this issue Aug 7, 2019 · 21 comments

Comments

@kellermaennchen
Copy link

Feature Request

Add another option like the --registry option to the linkerd install CLI command which specifies from which registry the prometheus image should be pulled

What problem are you trying to solve?

Air gapped installation: In secure On-Prem k8s installations internet access is not an option

How should the problem be solved?

By adding another option like the --registry option to the linkerd install CLI command

What do you want to happen? Add any considered drawbacks.
I want the prometheus image to be pulled from another (private) registry and not only from DockerHub

Any alternatives you've considered?

Add the prometheus image to the linkerd registry ("gcr.io/linkerd-io")

Is there another way to solve this problem that isn't as good a solution?
Add the prometheus image to the linkerd registry ("gcr.io/linkerd-io")

How would users interact with this feature?

Through the linkerd install CLI command

If you can, explain how users will be able to use this. Maybe some sample CLI
output?
linkerd install --registry some.registry --promregistry another.registry | kubectl apply -f -

@grampelberg
Copy link
Contributor

@ihcsim @alpeb I feel like the intent of --registry was to solve this exact problem, but I'm hesitant to have it override different defaults. I might be over optimizing, but I'd love to avoid adding one more configuration option. Any good ideas?

@alpeb
Copy link
Member

alpeb commented Aug 7, 2019

Sounds to me like a specific enough case to be addressed through a Helm variable or kustomize.

@ihcsim
Copy link
Contributor

ihcsim commented Aug 7, 2019

@kellermaennchen Sounds like you have access to GCR? If you access to GCR isn't limited to only gcr.io/linkerd-io, will using the GCR DockerHub mirror be an option for you? It should automatically redirect your request for the prom/prometheus image to the mirror.

Alternately, there is Helm and Kustomize, as @alpeb suggested.

The reason why we are hesitant to add yet another CLI option is so that we can maintain a sane list of configuration. Let us know if any of these alternatives will work for you.

@kellermaennchen
Copy link
Author

@ihcsim No I don't have access to GCR from my k8s cluster therefore the mirror is not an option. Because of that I used the --registry option and pointed it to my local registry. This worked fine except for the prometheus image.
If as @grampleberg stated the purpose of the --registry option was to support air gapped installations it is useless as long as you need access to DockerHub for the prometheus image. If you don't want to add another option to the CLI installation another solution could be to pull the prometheus image from the registry given by the --registry option as well.
Regarding the helm installation. I would prefer installation through helm over the CLI installation, as long as I do not have to tweak the original helm chart, because tracking and merging changes in the official and my custom helm chart could get a nightmare soon. As I am a total newby to Linkerd I just followed the the Getting Started guidelines. Is there some documentation about the helm inatallation?

@ihcsim
Copy link
Contributor

ihcsim commented Aug 8, 2019

@kellermaennchen The Helm chart landed in master couple days ago. It will be included in today's edge. Stay tuned! I'll love to hear your feedback.

With Helm, all you need to do is to modify the PrometheusImage variable either directly in the values.yaml file or use --set PrometheusImage=<my-registry>:<prom-version>.

@kellermaennchen
Copy link
Author

Thanks @ihcsim ! I will give it a try and share my feedback.

@kangxiaoning
Copy link

I have the same problem, I changed the installation command to linkerd install --registry local.registry | sed 's#prom/prometheus:v2.11.1#local.registry/linkerd-io/prometheus:v2.11.1#' | kubectl apply -f -.

@grampelberg
Copy link
Contributor

This would be a great first PR!

@christyjacob4
Copy link
Contributor

Hi @grampelberg I'd like to take this up

@cpretzer
Copy link
Contributor

cpretzer commented Feb 2, 2020

@christyjacob4 I think that you can implement this in the same way that the GrafanaImage value is set. Have a look at this diff.

installValues.GrafanaImage is set using string substitution with the value that is specified by the --registry option.

One way that the prometheus image is different from the grafana image is that the prometheus image uses a specific version, so you may need to take this in to consideration as well.

@christyjacob4
Copy link
Contributor

@cpretzer thank you for the insight .
Working on it now

@christyjacob4
Copy link
Contributor

@cpretzer How do you want the versioning of the prometheus image to be handled?
Can it default to the latest version ?
Could you give me an example of what the install command should look like after this feature is implemented?

@cpretzer
Copy link
Contributor

cpretzer commented Feb 9, 2020

@christyjacob4

We should continue to use the versioning pattern that we use now, which is to pin Prometheus to a particular version. Currently, that is 2.15.1: https://github.com/linkerd/linkerd2/blob/master/charts/linkerd2/values.yaml#L131

We pin the version because we want to ensure compatibility and stability, and we will continue to test against new versions of Prometheus as they are released.

With regard to the install command, have a look at the linkerd install -h command to see current flags that can be specified when installing Linkerd.

You'll see that there are a few of flags related to images: init-image, init-image-version, and proxy-image.

So, if we think through a solution that matches this pattern, it would be to add a flag like --prometheus-image. To expand on that, it's worth thinking through whether we need the image and version in one string, like the proxy-image flag, or whether it makes sense to be able to configure the prometheus image as well as the prometheus image version. At the moment, I can't think of a scenario where we would use anything other than a version of the official prometheus image, so it's probably safe to provide one string that includes the repository path and image version, just like proxy-image

Let me know if you have questions.

@christyjacob4
Copy link
Contributor

christyjacob4 commented Feb 9, 2020

Thank you @cpretzer That seems to clear a lot of my doubts. I read through the code for install command and I found usages of --registry (dockerRegistry) in root.go and install.go
So I'm guessing that these are the only files Ill need to modify .

Also, Since you mentioned about the syntax of the prometheus image, I was thinking of adding a regex Check in root.go along with the following condition

if options.prometheusRegistry != "" && !alphaNumDashDotSlashColon.MatchString(options.prometheusRegistry) {
		return fmt.Errorf("%s is not a valid Prometheus registry. The url can contain only letters, numbers, dash, dot, slash and colon", options.prometheusRegistry)
	}

Is that the expected way to do it? If yes, please give me an example of an input to --prometheus-image so that I can write a regex pattern .

Thanks again

@Pothulapati
Copy link
Contributor

@christyjacob4 Can you please create a draft PR on the work that you are planning, so that we can provide better feedback? :)

Also, I think we don't need to have a separate prometheusRegisry option, we can just reuse the registry flag that is passed and common across all the images. We should just enforce that for prometheusimage field too.

Once this is done, To make it more flexible we can have the prometheus-image flag as a installOption and will override the default as @cpretzer mentioned. :D

@christyjacob4
Copy link
Contributor

@Pothulapati Please take a look at my draft

@christyjacob4
Copy link
Contributor

@Pothulapati Regarding your suggestion, what if the user wants to fetch prometheus image from a different registry as opposed to grafana ?

@Pothulapati
Copy link
Contributor

@christyjacob4 that's a valid concern, but we can't keep adding registry flags for every image and for all use cases. They can always kustomize or other tools for unique/edge cases like that.

But supporting a common use case of having a registry flag that they can use for all images is important, and that's what we should be solving.

@christyjacob4
Copy link
Contributor

@Pothulapati Thanks for the clarification :)

@cpretzer
Copy link
Contributor

cpretzer commented Feb 12, 2020 via email

@christyjacob4
Copy link
Contributor

I think this can be closed in light of the merged PR

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants