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

Operator airgap mode support #14734

Closed
l0rd opened this issue Oct 1, 2019 · 13 comments
Closed

Operator airgap mode support #14734

l0rd opened this issue Oct 1, 2019 · 13 comments
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Milestone

Comments

@l0rd
Copy link
Contributor

l0rd commented Oct 1, 2019

Is your enhancement related to a problem?

A user that wants to install Che behind an enterprise firewall should be able to do so configuring the Checluster CR.

Describe the solution you'd like

Add the following CR attributes that a user can configure during installation of Che:

# airGapMode: 'true' ==> we won't need that (see comments below)
airGapContainerRegistryHostname: 'https://mycompany-registry/'
airGapContainerRegistryOrganisation: 'crw'

The default should be airGapMode: 'false'. When it's true the operator should use the specified hostname and repository to replace the images of the Che components, the plugins sidecars and the devfile registry stacks.

@l0rd l0rd added kind/enhancement A feature request - must adhere to the feature request template. team/osio area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator severity/P1 Has a major impact to usage or development of the system. labels Oct 1, 2019
@l0rd l0rd added this to the 7.3.0 milestone Oct 1, 2019
@tomgeorge
Copy link
Contributor

tomgeorge commented Oct 7, 2019

Status: added the CR fields, and will appropriately deploy all of the images based on airGapMode. Added jwtproxy, init plugin broker init, and init plugin broker unified images to the che configmap. I need to change release-operator-code.sh to get the correct image release tags for those images and write them to a .go file. You can see my latest work on https://github.com/tomgeorge/che-operator/tree/che-14734-2

I estimate that I would need ~ a half a day for the rest of the stuff.

@gorkem
Copy link
Contributor

gorkem commented Oct 8, 2019

I do not think “air gap” is a right term to use on the attribute names. How about ‘overrideContainerRegistryHostname‘?

@gorkem
Copy link
Contributor

gorkem commented Oct 8, 2019

Come to think of it, do we really need the Boolean airgapMode attribute ? If other host and repository attributes have some value they should go in effect.

@l0rd
Copy link
Contributor Author

l0rd commented Oct 8, 2019

@gorkem the 20 or so containers that are used when deploying Che and creating workspaces come from different registries (docker hub, quay.io, rhcc) and different repositories.

In other words for the non air gap use case we cannot just do

airGapContainerRegistryHostname: 'docker.io'
airGapContainerRegistryRepository: 'eclipse'

and we should rather

airGapContainerRegistryHostname: ""
airGapContainerRegistryRepository: ""

But I would rather explicitly set airGapMode: 'false' rather giving a semantic meaning to the empty strings.

@gorkem
Copy link
Contributor

gorkem commented Oct 9, 2019

Shouldn't these fields be optional on the CheCluster CRD? If they exist they override all the images otherwise they do not.

@davidfestal
Copy link
Contributor

They are optional yes.

@tomgeorge
Copy link
Contributor

tomgeorge commented Oct 9, 2019

Looking at @amisevsk's PRs for the devfile and plugin registries
eclipse-che/che-devfile-registry#110
eclipse-che/che-plugin-registry#245

They use the properties CHE_SIDECAR_CONTAINERS_REGISTRY_URL and
CHE_SIDECAR_CONTAINERS_REGISTRY_ORGANIZATION

For consistency, should we change the operator's air gap fields to airGapContainerRegistryUrl and airGapContainerRegistryOrganization?

@davidfestal
Copy link
Contributor

For consistency, should we change the operator's air gap fields to airGapContainerRegistryUrl and airGapContainerRegistryOrganization?

seems an interesting suggestion. @l0rd wdyt ?

@l0rd
Copy link
Contributor Author

l0rd commented Oct 9, 2019 via email

@amisevsk
Copy link
Contributor

amisevsk commented Oct 9, 2019

In the PRs I tried to choose appropriate env vars, but I'm happy to rework them if the naming is unclear. The current env vars are

# Plugin registry
CHE_SIDECAR_CONTAINERS_REGISTRY_URL
CHE_SIDECAR_CONTAINERS_REGISTRY_ORGANIZATION
CHE_SIDECAR_CONTAINERS_REGISTRY_TAG

# Devfile registry
CHE_DEVFILE_IMAGES_REGISTRY_URL
CHE_DEVFILE_IMAGES_REGISTRY_ORGANIZATION
CHE_DEVFILE_IMAGES_REGISTRY_TAG

In particular, HOSTNAME does seem to be a more standard term.

@tomgeorge
Copy link
Contributor

tomgeorge commented Oct 9, 2019

@l0rd @gorkem Are we keeping AirGapMode on the CR or just using the value of the Registry and Org not being empty strings to mean that we are in airgap mode?

@l0rd
Copy link
Contributor Author

l0rd commented Oct 10, 2019

@tomgeorge from a user point of view it's better if you remove AirGapMode on the CR. So if it doesn't make the code more complicated let's remove it.

@davidfestal
Copy link
Contributor

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-operator Issues and PRs related to Eclipse Che Kubernetes Operator kind/enhancement A feature request - must adhere to the feature request template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

5 participants