ability to install Operator with an alternate namcespace than the default (porter-operator-system) #191
Conversation
SubZer0MS
requested review from
carolynvs,
VinozzZ,
sgettys and
bdegeeter
as code owners
SubZer0MS
changed the title
|
https://getporter.org/contribute/guide/#signing-your-commits explains how to get the DCO check to pass |
Signed-off-by: Mihai Sarbulescu <mihai.sarbulescu@gmail.com>
Signed-off-by: Mihai Sarbulescu <mihai.sarbulescu@gmail.com>
SubZer0MS
force-pushed
the
mihsar_operator
branch
from
2cca919 to
4d32167
Compare
There was a problem hiding this comment.
Apologies for the slow review, I didn't see that you'd updated the PR after my last comment. It really helps to leave a comment on the PR when you make a change or push a commit that is ready for me to look at as it's easy to miss the notification that a new commit has been pushed to the PR.
I see the tests are not passing, let's get the current batch of comments addressed and see if that resolves it first.
There was a problem hiding this comment.
The configureNamespace function is used by the configureNamespace custom action for the bundle, it sets up a namespace for use with the operator with required configuration such as a porter configuration file. By default we use the contents of installer/manifests/namespace/defaults/porter-config-spec.yaml. In that file, we reference the in-cluster mongodb service that is installed in the operator namespace and it's hard-coded to porter-operator-system.
The configureNamespace helper function should also update that file when a custom porter configuration file was not specified to use the custom operator namespace.
storage:
- name: "in-cluster-mongodb"
plugin: "mongodb"
config:
url: "mongodb://mongodb.TODO-OPERATOR-NAMESPACE.svc.cluster.local"|
|
||
| setCustomNamespaceForOperator() { | ||
| if [ -z "$1" ]; then | ||
| echo "No namespace specified, using default $OPNAMESPACE" |
There was a problem hiding this comment.
Let's always be clear which namespace we are referring to in these messages, because we have both the "porter operator namespace" and also a namespace that is created and configured by the configureNamespace custom action in the bundle.
| echo "No namespace specified, using default $OPNAMESPACE" | |
| echo "No porter operator namespace specified, using default $OPNAMESPACE" |
| echo "No namespace specified, using default $OPNAMESPACE" | ||
| else | ||
| OPNAMESPACE=$1 | ||
| echo "Using custom namespace $OPNAMESPACE" |
There was a problem hiding this comment.
| echo "Using custom namespace $OPNAMESPACE" | |
| echo "Using custom porter operator namespace $OPNAMESPACE" |
| fi | ||
|
|
||
| # Replace the namespace in the operator.yaml | ||
| echo "Setting namespace to $OPNAMESPACE" |
There was a problem hiding this comment.
| echo "Setting namespace to $OPNAMESPACE" | |
| echo "Setting porter operator namespace to $OPNAMESPACE" |
| applyTo: | ||
| - install | ||
| - upgrade |
There was a problem hiding this comment.
The configureNamespace custom action will also need the porter operator namespace parameter.
| applyTo: | |
| - install | |
| - upgrade | |
| applyTo: | |
| - install | |
| - upgrade | |
| - configureNamespace |
| @@ -88,6 +95,12 @@ mixins: | |||
| - kubernetes | |||
|
|
|||
| install: | |||
| - exec: | |||
| description: "Set custom namespace for operator if present" | |||
There was a problem hiding this comment.
| description: "Set custom namespace for operator if present" | |
| description: "Set custom namespace for operator |
The parameter operatorNamespace will always be populated. So if it's not customized by the user it will just have the default value. Since this step always runs and the helper script always runs kustomize, let's make it clear that this logic always runs.
| @@ -1,6 +1,23 @@ | |||
| #!/usr/bin/env bash | |||
| set -euo pipefail | |||
|
|
|||
| OPNAMESPACE="porter-operator-system" | |||
There was a problem hiding this comment.
This works but I wanted you to know about a porter feature in case that affects how you'd like to implement this.
The parameter operatorNamespace is always populated, it will have the default value if not specified by the user, and a corresponding environment variable, OPERATORNAMESPACE set. There isn't a need to define OPNAMESPACE, pass the parameter as an argument to each helper function, and initialize OPNAMESPACE in each function. You can just use OPERATORNAMESPACE.
| @@ -334,7 +334,7 @@ func Deploy() { | |||
| buildPorterCmd("credentials", "apply", "hack/creds.yaml", "-n=operator").Must().RunV() | |||
| } | |||
| bundleRef := Env.BundlePrefix + meta.Version | |||
| installCmd := buildPorterCmd("install", "operator", "-r", bundleRef, "-c=kind", "--force", "-n=operator").Must() | |||
| installCmd := buildPorterCmd("install", "operator", "-r", bundleRef, "-c=kind", "--force", "-n=operator", "--param", "operatorNamespace="+operatorNamespace).Must() | |||
There was a problem hiding this comment.
Let's follow the pattern for allowing the developer to customize development parameters, like PORTER_AGENT_REPOSITORY, using local environment variables instead of requiring changes to the code.
The function applyHackParameters should be updated along with the hack/dev-build-params.yaml to set the operatorNamespace parameter from PORTER_OPERATOR_NAMESPACE on the local machine.
|
Heads up that mage build is failing. I've submitted a PR to get it working again and it's easy enough to patch locally to use debian stable until it's merged. |
What does this change
Added functionality of being able to specify an alternate namespace than the default (porter-operator-system) where the Operator is installed by passing in "--param operatorNamespace=operator-ns-name-here" to the operator porter install command.
What issue does it fix
Closes # (issue)
Notes for the reviewer
I'll handle the second issue which is discussed there (ability to install Operator with kustomize, without porter) in a separate PR.
Checklist