add nginx configurations for the mco-console #170
Conversation
| @@ -65,6 +65,13 @@ spec: | |||
| manageS3: | |||
| default: false | |||
| type: boolean | |||
| overlappingCIDR: | |||
There was a problem hiding this comment.
Why overlappingCIDR is part of this PR?
There was a problem hiding this comment.
is this missed, earlier ?
There was a problem hiding this comment.
yes, I think some other PR missed this... this is not due to this PR...
console/console.go
Outdated
| return nil | ||
| }); err != nil { | ||
| }); err != nil && !errors.IsAlreadyExists(err) { | ||
| // ConsolePlugin is a cluster-wide resource, and is not cleaned-up on operator un-install |
There was a problem hiding this comment.
it is CreateOrUpdate only, is this will raise an already exists error?
There was a problem hiding this comment.
I did not test this... but seems to me:
Let;s say I installed MCO for the first time >> operator will create ConsolePlugin CR for me.
Now, I deleted MCO operator >> ConsolePlugin CR did not get deleted as a part of operator or NameSpace deletion.
Now, I install MCO again >> operator will create ConsolePlugin again, but this time it will already be existing as it was not automatically cleaned up at time of operator deletion... it that case it will return "alreadyExists" error, and here I am ignoring that error.
There was a problem hiding this comment.
ok! but I did this multiple times, I never faced this issue, I think it was because of createOrUpdate, anyway you are gonna this.
There was a problem hiding this comment.
actually it make sense... u r right "createOrUpdate" will handle this... "IsAlreadyExists" check is not needed...
as discussed, leaving it as it is for now (that is MCO will be bringing both console pod and this ConfigMap)... |
console/console.go
Outdated
| @@ -42,6 +45,18 @@ var ( | |||
| serviceLabelKey = "app.kubernetes.io/name" | |||
| ) | |||
|
|
|||
| func getNginxConfConfigMap(namespace string) apiv1.ConfigMap { | |||
There was a problem hiding this comment.
| func getNginxConfConfigMap(namespace string) apiv1.ConfigMap { | |
| func getNginxConfConfigMap(namespace string) corev1.ConfigMap { |
package+version name
There was a problem hiding this comment.
just to point out, "apiv1" was used at multiple places in this file...
| overlappingCIDR: | ||
| default: false | ||
| description: OverlappingCIDR should be set to true if the peer clusters | ||
| have overlapping Pod or Service CIDR. This will enable storage clusters |
Once mco-console Deployment is deleted, corresponding Service should get deleted as well. Signed-off-by: SanjalKatiyar <[email protected]>
SanjalKatiyar
force-pushed
the
update_plugin_config
branch
from
c0f1120
to
18c22eb
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: SanjalKatiyar 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 |
SanjalKatiyar
force-pushed
the
update_plugin_config
branch
from
18c22eb
to
5e39faa
Compare
mco-console uses nginx for serving the UI assests. Add its configuration from the operator instead of build time. Signed-off-by: SanjalKatiyar <[email protected]>
SanjalKatiyar
force-pushed
the
update_plugin_config
branch
from
5e39faa
to
8002d0d
Compare
generated output of "make bundle" command. Signed-off-by: SanjalKatiyar <[email protected]>
@vbnrh don't we do it for controller's reconcile only ? we don't have specific controller for console, it run once during init. |
Not sure if it is added there. |
okay, I checked... we already have... |
|
/hold |
Check: red-hat-storage/odf-operator#305 for more information.