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

Fix installation defaults for Chectl and OLM files. #53

Merged
merged 2 commits into from
Jul 22, 2019

Conversation

davidfestal
Copy link
Contributor

This PR fixes a number of inconsistencies between OLM files (CSVs) and the main custom resource example used by CheCtl to start workspaces, so that installing from CheCtl, and from the OperatorHub would result in the same settings being applied.

Signed-off-by: David Festal <dfestal@redhat.com>
for the `nightly` channel of the openshift preview OLM package

Signed-off-by: David Festal <dfestal@redhat.com>
@@ -67,9 +71,9 @@ spec:
storage:
# persistent volume claim strategy for Che server. Can be common (all workspaces PVCs in one volume),
# per-workspace (one PVC per workspace for all declared volumes) and unique (one PVC per declared volume). Defaults to common
pvcStrategy: ''
pvcStrategy: 'per-workspace'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line above says - defaults to common.
And defaults.go actually has common as a default value https://github.com/eclipse/che-operator/blob/master/pkg/deploy/defaults.go#L32

There is an issue to use common PVC strategy as default eclipse-che/che#14614

@davidfestal @l0rd Seems common should be here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sleshchenko I assume I mainly kept what was set initially in the operator by Eugene, possibly in his initial CSV examples. TBH I don't remember. @l0rd wdyt ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say +1 for common but by default the namespace strategy is still one distinct per namespace per workspace right? How does common PVC strategy work with per-workspace namespace strategy?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say +1 for common but by default the namespace strategy is still one distinct per namespace per workspace right?

It depends on OAuth, if it's configured - is used, if it's not workspaces are run in the same namespace as Che Server.

How does common PVC strategy work with per-workspace namespace strategy?

Combination of:

  • common PVC + per workspace namespace
  • per workspace PVC + per workspace namespace

Works pretty the same the only difference PVC name that will be used - claim-che-workspace-<workspaceid>(for per-workspace) or claim-che-workspace(for common).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PVC strategy is changed in PR that introduce -che namespace by default 651c651
Probably OLM should be changed as well. Till add the corresponding TODO item in that PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants