-
Notifications
You must be signed in to change notification settings - Fork 152
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
Update docs and values.shema.json for operatorcrds migration #1619
base: main
Are you sure you want to change the base?
Conversation
b84bc7b
to
51a986f
Compare
51a986f
to
e7daef4
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.
Just some documentation nits, thanks for adding so much information with the change!
I'm not familiar enough with functionality to confidently sign off, but nothing looks off from what I understand.
|
||
#### Alternative Approach: Manual CRD Deployment | ||
|
||
If you prefer to manage CRD deployment manually, apply the CRDs using the commands below before installing the Helm chart: |
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.
How likely is it that these commands get outdated? Is there any way to make this into a test to ensure this doesn't go stale?
This is not a blocker for this PR, just something to consider.
|
||
#### CRD Cleanup | ||
|
||
When uninstalling this chart, the OpenTelemetry CRDs are not removed automatically. To delete them manually, use the following commands: |
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.
Same as previous comment, not blocking: Is there any way to ensure these commands stay up to date and work properly?
Co-authored-by: Curtis Robert <[email protected]>
Co-authored-by: Curtis Robert <[email protected]>
Co-authored-by: Curtis Robert <[email protected]>
curl -sL https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/main/config/crd/bases/opentelemetry.io_opampbridges.yaml | kubectl apply -f - | ||
curl -sL https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/main/config/crd/bases/opentelemetry.io_instrumentations.yaml | kubectl apply -f - | ||
``` | ||
|
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.
You can also use below helm template command to get the CRD yamls from the helm chart. This method can be helpful in keeping CRDs in-sync with the version bundled with our helm chart. | |
```bash | |
helm template helm-charts/splunk-otel-collector/ --include-crds \ | |
--set="splunkObservability.realm=us0,splunkObservability.accessToken=xxxxxx,clusterName=my-cluster,operatorcrds.install=true" \ | |
| yq e '. | select(.kind == "CustomResourceDefinition")' \ | |
| kubectl apply -f - |
We can direct users to leverage the our chart to get the crds directly, esp since they can have their CD pipelines just get the correct CRD version from the chart version they want to install
kubectl delete crd opampbridges.opentelemetry.io | ||
kubectl delete crd instrumentations.opentelemetry.io | ||
``` | ||
|
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.
You can use below combination of helm and kubectl command to delete CRDs. | |
```bash | |
helm template helm-charts/splunk-otel-collector/ --include-crds \ | |
--set="splunkObservability.realm=us0,splunkObservability.accessToken=xxxxxx,clusterName=my-cluster,operatorcrds.install=true" \ | |
| yq e '. | select(.kind == "CustomResourceDefinition")' \ | |
| kubectl delete --dry-run=client -f - |
- Users enabling the operator (`.Values.operator.enabled=true`) must now set `operatorcrds.install=true` in Helm values or [manually manage CRD installation](https://github.com/signalfx/splunk-otel-collector-chart/blob/main/docs/auto-instrumentation-install.md#crd-management). | ||
- Previously, CRDs were installed using templates (`operator.crds.create=true`), which could cause race conditions and installation failures. | ||
- CRD installation is now handled via Helm's native `crds/` directory for better stability, using a [localized subchart](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds). |
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.
- Users enabling the operator (`.Values.operator.enabled=true`) must now set `operatorcrds.install=true` in Helm values or [manually manage CRD installation](https://github.com/signalfx/splunk-otel-collector-chart/blob/main/docs/auto-instrumentation-install.md#crd-management). | |
- Previously, CRDs were installed using templates (`operator.crds.create=true`), which could cause race conditions and installation failures. | |
- CRD installation is now handled via Helm's native `crds/` directory for better stability, using a [localized subchart](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds). | |
- Users must set `operatorcrds.install=true` along with the existing option `operator.enabled=true` to install CRDs when deploying the opentelemetry-operator from this chart version onwards. | |
- CRD installation is now handled via Helm's native `crds/` directory for better stability, using a [localized subchart](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds). Refer to the [helm guide](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-1-let-helm-do-it-for-you) for further information about the new approach. | |
- Users trying to upgrade to this chart version with the operator already enabled (`operator.enabled=true`) can keep the installed CRDs by setting the arg `operator.crds.create=true`. The chart sets this to `false` by default in this version. This deprecated approach installs CRDs using templates which could cause race conditions and installation failures. Users are encouraged to reinstall with the new CRDs installation approach. |
Maybe something like this will work. Assuming we are removing the schema change for operator.crds.create.
|
||
#### Recommended Approach: Automated CRD Deployment | ||
|
||
Set the Helm chart value `operatorcrds.install=true` to allow the chart to handle CRD deployment automatically. |
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.
Set the Helm chart value `operatorcrds.install=true` to allow the chart to handle CRD deployment automatically. | |
Set the Helm chart value `operatorcrds.install=true` to allow the chart to handle CRD installation automatically. |
clarifies it is only useful on an install
#### Recommended Approach: Automated CRD Deployment | ||
|
||
Set the Helm chart value `operatorcrds.install=true` to allow the chart to handle CRD deployment automatically. | ||
_This option deploys the CRDs using a local subchart, available at [opentelemetry-operator-crds](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds)._ |
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.
_This option deploys the CRDs using a local subchart, available at [opentelemetry-operator-crds](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds)._ | |
_This option deploys the CRDs using a local subchart, available at [opentelemetry-operator-crds](https://github.com/signalfx/splunk-otel-collector-chart/tree/main/helm-charts/splunk-otel-collector/charts/opentelemetry-operator-crds)._ _Please note, helm will not update or delete these CRDs after initial install as noted in their [documentation](https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#some-caveats-and-explanations)._ |
Description: