Skip to content
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

Use a subchart to install CRDs via helm #1604

Merged
merged 4 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .chloggen/move-operator-crd-install-method.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix
# The name of the component, or a single word describing the area of concern, (e.g. agent, clusterReceiver, gateway, operator, chart, other)
component: operator
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Move operator CRD installation to the crds/ folder via a subchart to resolve Helm install ordering issues
# One or more tracking issues related to the change
issues: [1561]
# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

Large diffs are not rendered by default.

58 changes: 0 additions & 58 deletions functional_tests/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,41 +172,6 @@ func deployChartsAndApps(t *testing.T) {
}
}

// deploy the operator CRDs
stream, err = os.ReadFile(filepath.Join(testDir, manifestsDir, "operator_crds.yaml"))
require.NoError(t, err)

for _, resourceYAML := range strings.Split(string(stream), "---") {
if len(resourceYAML) == 0 {
continue
}

obj, groupVersionKind, err := decode(
[]byte(resourceYAML),
nil,
nil)
require.NoError(t, err)
if groupVersionKind.Group == "apiextensions.k8s.io" &&
groupVersionKind.Version == "v1" &&
groupVersionKind.Kind == "CustomResourceDefinition" {
crd := obj.(*appextensionsv1.CustomResourceDefinition)
apiExtensions := extensionsClient.ApiextensionsV1().CustomResourceDefinitions()
crd, err := apiExtensions.Create(context.Background(), crd, metav1.CreateOptions{})
require.NoError(t, err)
t.Logf("Deployed CRD %s", crd.Name)
for _, version := range crd.Spec.Versions {
sch.AddKnownTypeWithName(
schema.GroupVersionKind{
Group: crd.Spec.Group,
Version: version.Name,
Kind: crd.Spec.Names.Kind,
},
&unstructured.Unstructured{},
)
}
}
}

codecs := serializer.NewCodecFactory(sch)
crdDecode := codecs.UniversalDeserializer().Decode
// Prometheus pod monitor
Expand Down Expand Up @@ -536,29 +501,6 @@ func teardown(t *testing.T) {
}
}

crdstream, err = os.ReadFile(filepath.Join(testDir, manifestsDir, "operator_crds.yaml"))
require.NoError(t, err)
for _, resourceYAML := range strings.Split(string(crdstream), "---") {
if len(resourceYAML) == 0 {
continue
}

obj, groupVersionKind, err := decode(
[]byte(resourceYAML),
nil,
nil)
require.NoError(t, err)
if groupVersionKind.Group == "apiextensions.k8s.io" &&
groupVersionKind.Version == "v1" &&
groupVersionKind.Kind == "CustomResourceDefinition" {
crd := obj.(*appextensionsv1.CustomResourceDefinition)
apiExtensions := extensionsClient.ApiextensionsV1().CustomResourceDefinitions()
_ = apiExtensions.Delete(context.Background(), crd.Name, metav1.DeleteOptions{
GracePeriodSeconds: &waitTime,
})
}
}

for _, nm := range namespaces {
nmClient := client.CoreV1().Namespaces()
_ = nmClient.Delete(context.Background(), nm.Name, metav1.DeleteOptions{
Expand Down
2 changes: 0 additions & 2 deletions functional_tests/testdata/values/aks_test_values.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ livenessProbe:

operator:
enabled: true
crds:
create: false
admissionWebhooks:
certManager:
enabled: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ distribution: gke/autopilot

operator:
enabled: true
crds:
create: false
admissionWebhooks:
certManager:
enabled: false
Expand Down
2 changes: 0 additions & 2 deletions functional_tests/testdata/values/test_values.yaml.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ extraAttributes:
environment: dev
operator:
enabled: true
crds:
create: false
targetAllocator:
enabled: true

Expand Down
9 changes: 7 additions & 2 deletions helm-charts/splunk-otel-collector/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ maintainers:
- name: atoulme
dependencies:
# Subchart Notes:
# - Avoid uppercase letters in names/aliases, they cause install failure due to subchart resource naming
# - Avoid hyphen characters in names/aliases, they introduce template rendering complications (https://github.com/helm/helm/issues/2192)
# - Avoid uppercase letters in aliases, they cause install failure due to subchart resource naming
# - Avoid hyphen characters in aliases, they introduce template rendering complications (https://github.com/helm/helm/issues/2192)
- name: cert-manager
version: v1.14.4
alias: certmanager
repository: https://charts.jetstack.io
condition: certmanager.enabled
- name: opentelemetry-operator-crds
version: "0.0.0"
atoulme marked this conversation as resolved.
Show resolved Hide resolved
alias: operatorcrds
# TODO: See if we can improve this conditional more
condition: operatorcrds.install
- name: opentelemetry-operator
version: 0.71.2
alias: operator
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
apiVersion: v2
name: opentelemetry-operator-crds
version: 0.0.0
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# OpenTelemetry Collector CRDs

This chart contains the CRDs for _*installation*_ only right now for the opentelemetry-operator. This allows the Splunk OpenTelemetry Collector chart to work on install. You can see more discussion about this [here](https://github.com/open-telemetry/opentelemetry-helm-charts/issues/677) and [here](https://github.com/open-telemetry/opentelemetry-helm-charts/pull/1203).

This approach is inspired by the [opentelemetry-kube-stack chart](https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-kube-stack) approach.

> [!NOTE]
> The splunk-otel-collector chart only supports and tests functionality related to the auto-instrumentation that requires the Instrumentation CRD.
> Other CRDs, such as OpenTelemetryCollector and OpAMPBridge, are included solely to allow the Operator to start up and are not currently supported or tested.

# Upgrade Notes

Right now, upgrades are NOT handled by this chart, however that could change in the future. This is what is run to bring in the CRDs today.

```bash
wget https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/main/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml
wget https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/main/config/crd/bases/opentelemetry.io_opampbridges.yaml
wget https://raw.githubusercontent.com/open-telemetry/opentelemetry-operator/main/config/crd/bases/opentelemetry.io_instrumentations.yaml
```
Loading
Loading