Skip to content

[3.3] AutoOps: Ignore deprecated ES clusters (#9008) #9013

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.

Sign up for GitHub

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

Merged
naemono merged 1 commit into from
Jan 9, 2026
Merged

[3.3] AutoOps: Ignore deprecated ES clusters (#9008) #9013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 23 additions & 7 deletions pkg/controller/autoops/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/elastic/cloud-on-k8s/v3/pkg/controller/association"
"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/deployment"
"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/reconciler"
"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/version"
"github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s"
ulog "github.com/elastic/cloud-on-k8s/v3/pkg/utils/log"
)
Expand Down Expand Up @@ -134,16 +135,31 @@ func (r *AgentPolicyReconciler) internalReconcile(
for _, es := range accessibleClusters {
log := log.WithValues("es_namespace", es.Namespace, "es_name", es.Name)

esVersion, err := version.Parse(es.Spec.Version)
if err != nil {
log.Error(err, "while parsing ES version")
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
return results.WithError(err)
}

// No error means the version is within the deprecated range, so we skip the cluster.
// We do not adjust the status to indicate this issue at this time, as the status object
// does not currently support a status per-cluster.
if version.DeprecatedVersions.WithinRange(esVersion) == nil {
log.Info("Skipping ES cluster because of deprecated version", "version", es.Spec.Version)
Comment on lines +146 to +149
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When all matching ES clusters are deprecated, the status is left with Resources counted but no Phase set. This creates an ambiguous state where Resources=1, Ready=0, Errors=0, but Phase="". Consider setting an appropriate phase (or documenting this as expected behavior) to make the status more meaningful to users. For comparison, when resources exist but are not ready, the phase is explicitly set to ResourcesNotReadyPhase.

Suggested change
// We do not adjust the status to indicate this issue at this time, as the status object
// does not currently support a status per-cluster.
if version.DeprecatedVersions.WithinRange(esVersion) == nil {
log.Info("Skipping ES cluster because of deprecated version", "version", es.Spec.Version)
// We do not adjust the status to indicate this issue at this time on a per-cluster basis,
// but we do update the overall phase to indicate that resources are not in a usable state.
if version.DeprecatedVersions.WithinRange(esVersion) == nil {
log.Info("Skipping ES cluster because of deprecated version", "version", es.Spec.Version)
state.UpdateWithPhase(autoopsv1alpha1.ResourcesNotReadyPhase)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@naemono naemono Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this issue will likely be addressed when the extended status reporting issue is resolved/tackled

continue
}
Comment on lines +145 to +151
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecated ES clusters are included in the accessibleClusters list passed to cleanupOrphanedResourcesForPolicy, which prevents their associated resources from being cleaned up. However, these clusters are then skipped during reconciliation (line 150 continue), so no resources are actually created or maintained for them. This creates an inconsistency where old resources for deprecated clusters may remain in the cluster without being managed or cleaned up. Consider filtering deprecated clusters before passing to cleanupOrphanedResourcesForPolicy.

Copilot uses AI. Check for mistakes.

if es.Status.Phase != esv1.ElasticsearchReadyPhase {
log.V(1).Info("Skipping ES cluster that is not ready", "es_namespace", es.Namespace, "es_name", es.Name)
log.V(1).Info("Skipping ES cluster that is not ready")
state.UpdateWithPhase(autoopsv1alpha1.ResourcesNotReadyPhase)
results = results.WithRequeue(reconciler.DefaultRequeue)
continue
}

if es.Spec.HTTP.TLS.Enabled() {
if err := r.reconcileAutoOpsESCASecret(ctx, policy, es); err != nil {
log.Error(err, "while reconciling AutoOps ES CA secret", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while reconciling AutoOps ES CA secret")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand All @@ -153,7 +169,7 @@ func (r *AgentPolicyReconciler) internalReconcile(

apiKeySecret, err := r.reconcileAutoOpsESAPIKey(ctx, policy, es)
if err != nil {
log.Error(err, "while reconciling AutoOps ES API key", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while reconciling AutoOps ES API key")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand All @@ -162,7 +178,7 @@ func (r *AgentPolicyReconciler) internalReconcile(

configMap, err := ReconcileAutoOpsESConfigMap(ctx, r.Client, policy, es)
if err != nil {
log.Error(err, "while reconciling AutoOps ES config map", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while reconciling AutoOps ES config map")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand All @@ -171,7 +187,7 @@ func (r *AgentPolicyReconciler) internalReconcile(

configHash, err := buildConfigHash(ctx, *configMap, *apiKeySecret, r.Client, policy)
if err != nil {
log.Error(err, "while building config hash", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while building config hash")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand All @@ -180,7 +196,7 @@ func (r *AgentPolicyReconciler) internalReconcile(

deploymentParams, err := r.buildDeployment(configHash, policy, es)
if err != nil {
log.Error(err, "while getting deployment params", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while getting deployment params")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand All @@ -189,7 +205,7 @@ func (r *AgentPolicyReconciler) internalReconcile(

reconciledDeployment, err := deployment.Reconcile(ctx, r.Client, deploymentParams, &policy)
if err != nil {
log.Error(err, "while reconciling deployment", "es_namespace", es.Namespace, "es_name", es.Name)
log.Error(err, "while reconciling deployment")
errorCount++
state.UpdateWithPhase(autoopsv1alpha1.ErrorPhase)
results.WithError(err)
Expand Down
66 changes: 65 additions & 1 deletion pkg/controller/autoops/reconcile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) {
Status: esv1.ElasticsearchStatus{
Phase: esv1.ElasticsearchApplyingChangesPhase,
},
Spec: esv1.ElasticsearchSpec{
Version: "9.1.0",
},
},
},
wantStatus: autoopsv1alpha1.AutoOpsAgentPolicyStatus{
Expand Down Expand Up @@ -335,6 +338,9 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) {
Namespace: "ns-1",
Labels: map[string]string{"app": "elasticsearch"},
},
Spec: esv1.ElasticsearchSpec{
Version: "9.1.0",
},
Status: esv1.ElasticsearchStatus{
Phase: esv1.ElasticsearchApplyingChangesPhase,
},
Expand Down Expand Up @@ -548,6 +554,64 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) {
},
wantResults: reconcile.Result{},
},
{
name: "deprecated ES version is ignored",
policy: autoopsv1alpha1.AutoOpsAgentPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "policy-1",
Namespace: "ns-1",
},
Spec: autoopsv1alpha1.AutoOpsAgentPolicySpec{
Version: "9.2.1",
AutoOpsRef: autoopsv1alpha1.AutoOpsRef{
SecretName: "config-secret",
},
ResourceSelector: metav1.LabelSelector{
MatchLabels: map[string]string{"app": "elasticsearch"},
},
},
},
initialObjects: []client.Object{
&corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "config-secret",
Namespace: "ns-1",
},
Data: map[string][]byte{
"cloud-connected-mode-api-key": []byte("test-key"),
"autoops-otel-url": []byte("https://test-url"),
"autoops-token": []byte("test-token"),
},
},
&esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Name: "es-deprecated",
Namespace: "ns-1",
Labels: map[string]string{"app": "elasticsearch"},
},
Spec: esv1.ElasticsearchSpec{
Version: "7.15.0",
HTTP: commonv1.HTTPConfig{
TLS: commonv1.TLSOptions{
SelfSignedCertificate: &commonv1.SelfSignedCertificate{
Disabled: true,
},
},
},
},
Status: esv1.ElasticsearchStatus{
Phase: esv1.ElasticsearchReadyPhase,
},
},
},
wantStatus: autoopsv1alpha1.AutoOpsAgentPolicyStatus{
Resources: 1,
Ready: 0,
Errors: 0,
Phase: "",
},
wantResults: reconcile.Result{},
},
}

for _, tt := range tests {
Expand Down Expand Up @@ -577,7 +641,7 @@ func TestAutoOpsAgentPolicyReconciler_internalReconcile(t *testing.T) {

gotResult, gotErr := gotResults.Aggregate()
expectError := tt.wantStatus.Phase == autoopsv1alpha1.ErrorPhase || tt.wantStatus.Phase == autoopsv1alpha1.InvalidPhase
require.Equal(t, expectError, gotErr != nil)
require.Equal(t, expectError, gotErr != nil, "expected error: %v, got error: %v", expectError, gotErr)

if !cmp.Equal(tt.wantStatus, state.status, cmpopts.IgnoreFields(autoopsv1alpha1.AutoOpsAgentPolicyStatus{}, "ObservedGeneration")) {
t.Errorf("status mismatch:\n%s", cmp.Diff(tt.wantStatus, state.status, cmpopts.IgnoreFields(autoopsv1alpha1.AutoOpsAgentPolicyStatus{}, "ObservedGeneration")))
Expand Down
Loading