-
Notifications
You must be signed in to change notification settings - Fork 2
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
First release #1
Conversation
Signed-off-by: Jordi Gil <jgil@redhat.com>
…e helm operator Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
…e backstage CRD to be provided by this operator Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
… all instances of wait for jobs to use this new approach Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
…plate Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
… hook Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
6367277
to
031c9eb
Compare
Signed-off-by: Jordi Gil <jgil@redhat.com>
ddb3426
to
f8b64ba
Compare
… recreate CSV Signed-off-by: Jordi Gil <jgil@redhat.com>
f8b64ba
to
264d98e
Compare
Signed-off-by: Jordi Gil <jgil@redhat.com>
…-operator.v0.0.1 for name\n* Use openshift-operators for namespace Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
Signed-off-by: Jordi Gil <jgil@redhat.com>
2352ee4
to
322ede8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running the helm chart in a non-devmode, there is a need to create the namespace and it is also expected to create the DB.
in devmode, the chart is responsible to create the namespace CR.
AFAIKT, we don't maintain the condition of the CR - how that kind of errors (missing namespace) are propagated to the user?
other than that, this looks pretty good.
@@ -0,0 +1,7 @@ | |||
# Build the manager binary | |||
FROM quay.io/jordigilh/helm-operator:dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you like to replace this with the orchestartor organization on quay?
we can also move this repository to parodos-dev github org and use its Quay secret for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect this to be replaced by the helm-operator container image in the next release of the operator-sdk now that this PR has been merged.
# To re-generate a bundle for another specific version without changing the standard setup, you can: | ||
# - use the VERSION as arg of the bundle target (e.g make bundle VERSION=0.0.2) | ||
# - use environment variables to overwrite this value (e.g export VERSION=0.0.2) | ||
VERSION ?= 0.0.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this version or any version be aligned with the orchestrator helm chart version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes if we plan to release the operator when new a new chart is released, regardless of its correctness. But I'd be careful about that since we would be risking releasing a version that contains a chart that may not work with the operator before we have tested it.
A reason for not coupling them is that there might be improvements to the helm operator (update the base container image for instance) not tied to the helm chart that would require a release of the chart without changes just to align with the version in the operator.
a0e4565
to
e266f29
Compare
* Renamed operator to "Orchestrator Operator" * Removed references in the name to Parodos * Rebuilt using operator-sdk v1.33 because of an issue in the make bundle operation in v1.34.1 (operator-framework/operator-sdk#6698) * Grouped roles for namespace into a single entry Signed-off-by: Jordi Gil <jgil@redhat.com>
e266f29
to
d7ee0f5
Compare
We could add the check for the namespace to be there and fail otherwise. WDYT? |
Pls note operator-framework/operator-sdk#6691 was merged. |
Makefile
Outdated
# For example, running 'make bundle-build bundle-push catalog-build catalog-push' will build and push both | ||
# parodos.dev/operator-bundle:$VERSION and parodos.dev/operator-catalog:$VERSION. | ||
IMAGE_TAG_BASE ?= quay.io/parodos-dev/orchestrator-controller | ||
IMAGE_TAG_BASE ?= quay.io/orchestrator/controller-manager |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use orchestrator-operator
as image name instead? if this is the default name generated by the SDK, and has a wider effect on the rest of the generated code, we can use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's defined here:
https://github.com/jordigilh/orchestrator-helm-operator/blob/d7ee0f512ce88cfe76e79b216a184d6af6a86357/PROJECT#L10-L12
We can set the value we want anyway. I'll change it to orchestrator-operator
as suggested.
@@ -48,7 +46,7 @@ endif | |||
|
|||
# Set the Operator SDK version to use. By default, what is installed on the system is used. | |||
# This is useful for CI or a project to utilize a specific version of the operator-sdk toolkit. | |||
OPERATOR_SDK_VERSION ?= v1.34.1 | |||
OPERATOR_SDK_VERSION ?= v1.33.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a lower version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There seems to be an issue with 1.34.1 on running the bundle target (make bundle
) that produces incomplete artifacts:
operator-framework/operator-sdk#6698
I reverted to use the previously available version (1.33.0) to avoid the issue mentioned.
…nvention to other references using controller-manager as name Signed-off-by: Jordi Gil <jgil@redhat.com>
First release of the helm operator temporally aligned to chart version in this PR
parodos-dev/orchestrator-helm-chart#111
Uses a custom helm operator container image that includes this PR
operator-framework/operator-sdk#6691
which is needed to ensure that the dry-runs check for server side resources when the operator evaluates upgrade actions based on changes to the objects stored in the API server.
Note that in terms of roles, this operator requires quite a significant amount of access. The list was composed by capturing the failures to access resources with given verbs in the logs when running the operator.
Since the orchestrator CR can be deployed in any namespace, it is not possible to limit the access required by the controller to certain namespaces via
Role/RoleBinding
.@masayag please take a look.