You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/11/08 12:09:44 UTC
[GitHub] [camel-k] squakez opened a new pull request #2735: feat(cmd): OLM default discovery
squakez opened a new pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735
If the cluster which we are connected is a plain K8S cluster, we will use Operator SDK community parameters during OLM installation procedure.
With this PR we're discovering the cluster type dynamically and provide the proper OLM configuration during install/uninstall phase.
Closes #2113
<!-- Description -->
<!--
Enter your extended release note in the below block. If the PR requires
additional action from users switching to the new release, include the string
"action required". If no release note is required, write "NONE".
You can (optionally) mark this PR with labels "kind/bug" or "kind/feature" to make sure
the text is added to the right section of the release notes.
-->
**Release Note**
```release-note
feat(cmd): OLM default discovery
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
squakez commented on pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#issuecomment-965174718
The E2E are failing because:
```
--- FAIL: TestMetrics/reconciliation_duration_metric (0.04s)
```
not related to this PR and reported in #2742. I am going to merge if there is no objection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez merged pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
squakez merged pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#discussion_r744760611
##########
File path: pkg/util/olm/operator.go
##########
@@ -343,17 +345,31 @@ func fillDefaults(o Options) Options {
if o.Channel == "" {
o.Channel = DefaultChannel
}
- if o.Source == "" {
- o.Source = DefaultSource
- }
- if o.SourceNamespace == "" {
- o.SourceNamespace = DefaultSourceNamespace
- }
if o.StartingCSV == "" {
o.StartingCSV = DefaultStartingCSV
}
- if o.GlobalNamespace == "" {
- o.GlobalNamespace = DefaultGlobalNamespace
+ if isOCP, err := openshift.IsOpenShift(client); isOCP && err != nil {
Review comment:
I did not want to change the function signature, neither ignore the `err` return value. The idea is that if an error is returned, then we always default to the community configuration. Having a look at the `openshift.IsOpenShift` I don't expect any kind of error though. And if there is any cluster error, then we should not reach this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#discussion_r744743541
##########
File path: pkg/util/olm/operator.go
##########
@@ -343,17 +345,31 @@ func fillDefaults(o Options) Options {
if o.Channel == "" {
o.Channel = DefaultChannel
}
- if o.Source == "" {
- o.Source = DefaultSource
- }
- if o.SourceNamespace == "" {
- o.SourceNamespace = DefaultSourceNamespace
- }
if o.StartingCSV == "" {
o.StartingCSV = DefaultStartingCSV
}
- if o.GlobalNamespace == "" {
- o.GlobalNamespace = DefaultGlobalNamespace
+ if isOCP, err := openshift.IsOpenShift(client); isOCP && err != nil {
Review comment:
I'm not sure I understand the intent of `err != nil`. It seems the error should be returned instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] squakez commented on a change in pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
squakez commented on a change in pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#discussion_r744877760
##########
File path: pkg/util/olm/operator.go
##########
@@ -343,17 +345,31 @@ func fillDefaults(o Options) Options {
if o.Channel == "" {
o.Channel = DefaultChannel
}
- if o.Source == "" {
- o.Source = DefaultSource
- }
- if o.SourceNamespace == "" {
- o.SourceNamespace = DefaultSourceNamespace
- }
if o.StartingCSV == "" {
o.StartingCSV = DefaultStartingCSV
}
- if o.GlobalNamespace == "" {
- o.GlobalNamespace = DefaultGlobalNamespace
+ if isOCP, err := openshift.IsOpenShift(client); isOCP && err != nil {
Review comment:
I originally though it involved bubbling up the error into too many levels. Turned out it was not the case as we already manage an error condition in the same calling method. Resolved via 58dc6351eebb9a59f58b42adbe16a04c9e8aef4c
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#discussion_r744767177
##########
File path: pkg/util/olm/operator.go
##########
@@ -343,17 +345,31 @@ func fillDefaults(o Options) Options {
if o.Channel == "" {
o.Channel = DefaultChannel
}
- if o.Source == "" {
- o.Source = DefaultSource
- }
- if o.SourceNamespace == "" {
- o.SourceNamespace = DefaultSourceNamespace
- }
if o.StartingCSV == "" {
o.StartingCSV = DefaultStartingCSV
}
- if o.GlobalNamespace == "" {
- o.GlobalNamespace = DefaultGlobalNamespace
+ if isOCP, err := openshift.IsOpenShift(client); isOCP && err != nil {
Review comment:
Is there any reason why you don't want to change the function signature? I'd rather keep it idiomatic and fail fast, instead of introducing that pattern that is hard to read for the external developer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-k] astefanutti commented on a change in pull request #2735: feat(cmd): OLM default discovery
Posted by GitBox <gi...@apache.org>.
astefanutti commented on a change in pull request #2735:
URL: https://github.com/apache/camel-k/pull/2735#discussion_r744767177
##########
File path: pkg/util/olm/operator.go
##########
@@ -343,17 +345,31 @@ func fillDefaults(o Options) Options {
if o.Channel == "" {
o.Channel = DefaultChannel
}
- if o.Source == "" {
- o.Source = DefaultSource
- }
- if o.SourceNamespace == "" {
- o.SourceNamespace = DefaultSourceNamespace
- }
if o.StartingCSV == "" {
o.StartingCSV = DefaultStartingCSV
}
- if o.GlobalNamespace == "" {
- o.GlobalNamespace = DefaultGlobalNamespace
+ if isOCP, err := openshift.IsOpenShift(client); isOCP && err != nil {
Review comment:
Is there any reason why you don't want to change the function signature? I'd rather keep it idiomatic and fail fast, instead of introducing that pattern, that is hard to read for the external developer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org