LoggingLogMetric direct actuation checklist

[acpana]

Copied as a worksheet from #1681 but changed it a fair amount

Checklist for LoggingLogMetric (LLM) direct actuation. There may be some overlap on items. Not all of these will necessarily be applicable.

Checklist for existing resources

Code & Reconcilliaiton

• Review directbase_controller for todos
• Audit mapping logic - e.g. fields should have a if foo != nil check
• Verify that mutable-but-unreadable fields don't cause (infinite) diffs
• Verify that any custom diffs from the DCL controller are reflected in the direct path

KCC System

• Double check that the user agent is set as Kcc/controller-manager (consistent with other controllers) in order for usage telemetry to work

CRD

• make sure the CRD is backwards compatible

Special Labels/ Directives support

• check that the "special" directives here continue to be supported

General Labels/ Directives support

As taken from: https://github.com/maqiuyujoyce/k8s-config-connector/blob/master/pkg/k8s/constants.go

• mutable-but-unreadable-fields
• observed-secret-versions
• supports-ssa
• blueprint
• management-conflict-prevention-policy
• deletion-policy
• reconcile-interval-in-seconds

Container Annotations

• project-id
• folder-id
• organization-id

Functional

• pause via CC, CCC or annotations works
• unmanaging works
• acquisition of resource works
• abandon on delete works
• cascading deletes

References

• Check that references in work
• Check that references out work
• Check that IAM references work

Immutability

• check that fields that were immutable before, continue to be immutable

Webhooks

• check the deletion defender still works
• check that the immutability webhook still works

Testing

• Create tests for any "difficult" scenarios
• Create tests
  • Field testing
    • Field set, unmanage and unset

---

Enchancements

• Redacted log for why we are diffing
• Redacted log for rest requests
• enh: add metrics to (direct) controller(s) #1858
  • reconcile success rate per kind
  • reconcile time per kind
  • work queue length

[acpana]

• Review directbase_controller for todos

Calling this done as remaining TODOs are either not applicable to LLM specifically for its parity b/w a DCL and direct controller or contain simple refactor work like #1702

[acpana]

Calling this "done" as merged in #1649 and spike proves that we are at parity b/w the DCL and direct version for the behavior of field set, unmanage, unset here.

• Field set, unmanage and unset

[acpana]

• Audit mapping logic - e.g. fields should have a if foo != nil check

AFAICT, all the fields have a check for unset intent or are at parity with the DCL controller

[acpana]

• observed-secret-versions

AFAICT, there are no sensitive fields in LoggingLogMetric BUT the directbase controller doesn't handle sensitive fields just yet. #1724 to track its addition

[acpana]

• acquisition of resource works

Calling this done as per #1741 (comment)

[acpana]

Container Annotations

• project-id
• folder-id
• organization-id

LLM (dcl based) does not support container annotations as there would be an SupportsContainerAnnotations: true, in the following:

k8s-config-connector/pkg/dcl/metadata/metadata.go

Lines 527 to 531 in 1b48ee4

	{
	Kind: "LoggingLogMetric",
	Releasable: true,
	SupportsHierarchicalReferences: true,
	},

[acpana]

• deletion-policy

directbase_controller already honors the deletion-policy annotation:

k8s-config-connector/pkg/controller/direct/directbase/directbase_controller.go

Lines 230 to 246 in 1b48ee4

	if !k8s.HasAbandonAnnotation(u) {
	if _, err := adapter.Delete(ctx); err != nil {
	if !errors.Is(err, kcciamclient.ErrNotFound) && !k8s.IsReferenceNotFoundError(err) {
	if unwrappedErr, ok := lifecyclehandler.CausedByUnresolvableDeps(err); ok {
	logger.Info(unwrappedErr.Error(), "resource", k8s.GetNamespacedName(u))
	resource, err := toK8sResource(u)
	if err != nil {
	return false, fmt.Errorf("error converting k8s resource while handling unresolvable dependencies event: %w", err)
	}
	// Requeue resource for reconciliation with exponential backoff applied
	return true, r.Reconciler.HandleUnresolvableDeps(ctx, resource, unwrappedErr)
	}
	return false, r.handleDeleteFailed(ctx, u, err)
	}
	}
	}
	return false, r.handleDeleted(ctx, u)

For future conversion we can probably get rid of this requirement or maybe just relegate it to a adding a test!

[acpana]

• cascading deletes

checked by deleting the (new) CRD for LLM

[gemmahou]

• check that fields that were immutable before, continue to be immutable
  PR: feat: Support immutability checks for llm #1750

Add immutability check in llm controller, for fields: spec.metricDescriptor.metricKind and valueType; Add scenario test cases to verify that the direct controller behaves the same as DCL.

[gemmahou]

• check that the immutability webhook still works

PR#1750 also add immutability checks for llm controller in deny-immutable-field-updates webhook

Local test steps:

1. Create a KRM resource, describe the resource verify it's up to date
2. Update an immutable field and apply
3. The operation fails as expected, example console output: https://screenshot.googleplex.com/3rf8557eTCqx2Fn.png

• check the deletion defender still works

Local test steps:

1. Create a KRM resource, describe the resource verify it's up to date and check it's created in Cloud Console
2. Delete the CRD logginglogmetrics.logging.cnrm.cloud.google.com
3. Verify that the KRM resource is gone, but GCP resource still exists in Cloud Console

[acpana]

• reconcile-interval-in-seconds

Added as part of #1746

[acpana]

• abandon on delete works

Verified in conjunction with deletion defender when uninstalling (by deleting CRD).

[acpana]

CRD

• make sure the CRD is backwards compatible

Reviewed changes to CRD w team members and I ran over a few manual upgrade scenarios that involved:

• installing a pre 1.118 CRD for LoggingLogMetirc
• creating LoggingLogMetric resources
• updating the kcc control plane to sha c71bec6fb (post 1.117 release)
• installing the new CRD for LoggingLogMetric
• check controller logs, hand off successful
• modifying existing resources, see changes applied onto GCP/ cloud provider.

[acpana]

• pause via CC, CCC or annotations works

manual verification and pause-tests pass for this resource!

[acpana]

Special Labels/ Directives support

• check that the "special" directives here continue to be supported

As of today, the only "special" annotation is state-into-spec. For this resource, we have determined that all fields that would support state-into-spec: merge can be retrieved from the status.observedState. So for LLM as a direct controller resource, the annotation will be ignored.

[acpana]

• management-conflict-prevention-policy

This seems to be a TF based construct and would not apply to DCL based resources (which LLM is at the moment).

[acpana]

KCC System

• Double check that the user agent is set as Kcc/controller-manager (consistent with other controllers) in order for usage telemetry to work

We set the KCCUserAgent at registration time in the manager:

k8s-config-connector/pkg/controller/kccmanager/kccmanager.go

Line 139 in 5937271

UserAgent: gcp.KCCUserAgent,

[acpana]

Primarily via #1772 and other work around using the GCP updateTime, we protect against:

• Verify that mutable-but-unreadable fields don't cause (infinite) diffs

This is now being handled in the logmetric_controller:

k8s-config-connector/pkg/controller/direct/logging/utils.go

Lines 234 to 237 in 58f7154

	// this is a quirk of the API where the "STRING" default value gets returned as "".
	if ValueOf(kccLabels[i].ValueType) == "" {
	*kccLabels[i].ValueType = "STRING" // "" defaults to "STRING"
	}

• Verify that any custom diffs from the DCL controller are reflected in the direct path

Insofar as backwards compat and testing tough scenarios, between the work to add the dynamic controller for LoggingLogMetric as a direct resource and the step/ scenario tests, I called these testing section done too.

[acpana]

• unmanaging works

needs further discussion

the following were not added as labels

• mutable-but-unreadable-fields
  using the GCP update time to avoid spurious updates
• supports-ssa
  seems not needed anymore, see feat: trigger ssa metadata for direct controller #1745
• blueprint
  actively removing code paths for this: refactor: remove blueprint anon #1830

[acpana]

The enhancements section will be made as its own issue.