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

feat: Add support to install che using stable-all-namespaces olm channel #1478

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

flacatus
Copy link
Collaborator

@flacatus flacatus commented Jul 28, 2021

Signed-off-by: Flavius Lacatusu flacatus@redhat.com

What does this PR do?

Add support for stable-all-namespaces channel.

eclipse-che/che-operator#890

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Issue: eclipse-che/che#19891

How to test this PR?

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

src/api/context.ts Outdated Show resolved Hide resolved
await kube.deleteOperatorSubscription(SUBSCRIPTION_NAME, flags.chenamespace)
task: async (ctx: any, task: any) => {
const checlusters = await kube.getAllCheClusters()
if (kube.operatorSubscriptionExists(SUBSCRIPTION_NAME, DEFAULT_OPENSHIFT_OPERATORS_NS_NAME) && checlusters.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

kube.operatorSubscriptionExists(SUBSCRIPTION_NAME, DEFAULT_OPENSHIFT_OPERATORS_NS_NAME) && checlusters.length === 0

Maybe this check should be moved to the separated method, cause it was used not once?

Copy link
Collaborator

Choose a reason for hiding this comment

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

pls revert

task: async (ctx: any, task: any) => {
const checlusters = await kube.getAllCheClusters()
if (kube.operatorSubscriptionExists(SUBSCRIPTION_NAME, DEFAULT_OPENSHIFT_OPERATORS_NS_NAME) && checlusters.length === 0) {
await kube.deleteOperatorSubscription(SUBSCRIPTION_NAME, ctx.operatorNamespace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but you completely removed case with delete operator subscription for one specific namespace. And the same for Delete(OLM) Eclipse Che cluster service versions

@flacatus flacatus force-pushed the stable_allns branch 2 times, most recently from a817d95 to fcd4a21 Compare August 2, 2021 09:20
@@ -296,6 +296,10 @@ export default class Deploy extends Command {
this.error(`🛑 The specified installer ${flags.installer} does not support Minishift`)
}

if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME && (flags.platform === 'minikube' || flags.platform === 'k8s')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME && (flags.platform === 'minikube' || flags.platform === 'k8s')) {
if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME && isKubernetesPlatformFamily(flags.platform)) {

@flacatus flacatus force-pushed the stable_allns branch 2 times, most recently from f619c9f to 954a3e5 Compare August 2, 2021 11:14
@flacatus flacatus changed the title feat: Add support to install new channel using chectl cli feat: Add support to install che using stable-all-namespaces olm channel Aug 2, 2021
@@ -68,6 +68,8 @@ export class OLMTasks {
},
{
title: 'Create operator group',
// Not need it to create operator group in openshift-operators ns
enabled: () => flags['olm-channel'] !== STABLE_ALL_NAMESPACES_CHANNEL_NAME,
Copy link
Collaborator

@tolusha tolusha Aug 2, 2021

Choose a reason for hiding this comment

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

check operator namespace (adjust comment as well)

task.title = `${task.title}...done.`
},
},
{
title: 'Set custom operator image',
enabled: () => flags['che-operator-image'],
task: async (_ctx: any, task: any) => {
const csvList = await kube.getClusterServiceVersions(flags.chenamespace)
const csvList = await kube.getClusterServiceVersions(flags.chenamespaces)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo ?

} else if (VersionHelper.isDeployingStableVersion(flags) || flags['olm-channel'] === OLM_STABLE_CHANNEL_NAME) {
// stable Che CatalogSource
subscription = this.constructSubscription(SUBSCRIPTION_NAME, DEFAULT_CHE_OLM_PACKAGE_NAME, flags.chenamespace, ctx.defaultCatalogSourceNamespace, OLM_STABLE_CHANNEL_NAME, ctx.catalogSourceNameStable, ctx.approvalStarategy, ctx.startingCSV)
subscription = this.constructSubscription(SUBSCRIPTION_NAME, DEFAULT_CHE_OLM_PACKAGE_NAME, ctx.operatorNamespace, ctx.defaultCatalogSourceNamespace, OLM_STABLE_CHANNEL_NAME, ctx.catalogSourceNameStable, ctx.approvalStarategy, ctx.startingCSV)
} else if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME && isOpenshiftPlatformFamily(flags.platform)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
} else if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME && isOpenshiftPlatformFamily(flags.platform)) {
} else if (flags['olm-channel'] === STABLE_ALL_NAMESPACES_CHANNEL_NAME) {

We already checked it in deploy command

Signed-off-by: Flavius Lacatusu <flacatus@redhat.com>
@flacatus
Copy link
Collaborator Author

flacatus commented Aug 2, 2021

/retest

1 similar comment
@flacatus
Copy link
Collaborator Author

flacatus commented Aug 3, 2021

/retest

ctx.operatorNamespace = DEFAULT_OPENSHIFT_OPERATORS_NS_NAME
task.title = `${task.title}...Found`
}
ctx.operatorNamespace = flags.chenamespace
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ctx.operatorNamespace = flags.chenamespace
ctx.operatorNamespace = flags.chenamespace || DEFAULT_CHE_NAMESPACE

task: async (ctx: any, task: any) => {
if (await kube.operatorSubscriptionExists(SUBSCRIPTION_NAME, DEFAULT_OPENSHIFT_OPERATORS_NS_NAME)) {
ctx.operatorNamespace = DEFAULT_OPENSHIFT_OPERATORS_NS_NAME
task.title = `${task.title}...Found`
Copy link
Collaborator

Choose a reason for hiding this comment

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

return or else

await kube.deleteOperatorSubscription(SUBSCRIPTION_NAME, flags.chenamespace)
task: async (ctx: any, task: any) => {
const checlusters = await kube.getAllCheClusters()
if (kube.operatorSubscriptionExists(SUBSCRIPTION_NAME, DEFAULT_OPENSHIFT_OPERATORS_NS_NAME) && checlusters.length === 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

pls revert

Signed-off-by: Flavius Lacatusu <flacatus@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Aug 3, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: flacatus, tolusha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tolusha
Copy link
Collaborator

tolusha commented Aug 3, 2021

/retest

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Aug 3, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm label Aug 3, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #1478 (7da2773) into main (cd7c050) will increase coverage by 0.00%.
The diff coverage is 6.66%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1478   +/-   ##
=======================================
  Coverage   10.85%   10.85%           
=======================================
  Files          63       63           
  Lines        6504     6521   +17     
  Branches     1096     1102    +6     
=======================================
+ Hits          706      708    +2     
- Misses       5798     5813   +15     
Impacted Files Coverage Δ
src/api/kube.ts 5.59% <0.00%> (-0.01%) ⬇️
src/commands/server/deploy.ts 0.00% <0.00%> (ø)
src/tasks/installers/olm.ts 0.00% <0.00%> (ø)
src/api/context.ts 53.65% <25.00%> (-4.24%) ⬇️
src/constants.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd7c050...7da2773. Read the comment docs.

@flacatus flacatus merged commit 7f98e5c into main Aug 3, 2021
@flacatus flacatus deleted the stable_allns branch August 3, 2021 13:52
@che-bot che-bot added this to the 7.35 milestone Aug 3, 2021
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.

None yet

5 participants