Skip to content

Add conditions to httpbin CRs#31

Merged
cnvergence merged 5 commits intomainfrom
conditions-region-feat
Sep 29, 2025
Merged

Add conditions to httpbin CRs#31
cnvergence merged 5 commits intomainfrom
conditions-region-feat

Conversation

@cnvergence
Copy link
Copy Markdown
Collaborator

@cnvergence cnvergence commented Sep 24, 2025

https://github.com/platform-mesh/httpbin-operator/issues/61

Summary by CodeRabbit

  • New Features

    • Introduces standardized status conditions for HttpBin and HttpBinDeployment and exposes a Conditions list in status/CRDs for clearer readiness, URL and failure reporting.
    • Controllers now emit informative condition updates for creation, readiness, not-ready, endpoint/URL-ready, and failure events.
  • Bug Fixes

    • Fixed deep-copy of status conditions to prevent unintended mutations and improved status update reliability and logging.
  • Chores

    • Upgraded tooling to Kubebuilder v4.9.0.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 24, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds Kubernetes-style Conditions to HttpBin and HttpBinDeployment types and statuses, updates deepcopy generation, modifies controllers to set/update status.conditions during reconciliation, and bumps Kubebuilder/controller-gen tooling and CRD annotations/schemas to include the new status.conditions field.

Changes

Cohort / File(s) Summary
API: HttpBin types
api/v1alpha1/httpbin_types.go
Add HttpBinConditionType and HttpBinConditionReason types and constants (Ready, DeploymentReady, DeploymentProgressing, DeploymentFailed); add Conditions []metav1.Condition to HttpBinStatus.
API: HttpBinDeployment types
api/v1alpha1/httpbindeployment_types.go
Add HttpBinDeploymentConditionType and HttpBinDeploymentConditionReason types and multiple condition/reason constants (Ready, ServiceExposed, HttpBinInstanceCreated/Progressing/Failed, EndpointReady, ServiceExposureFailed).
Generated deepcopy
api/v1alpha1/zz_generated.deepcopy.go
Update deep-copy logic: HttpBin.DeepCopyInto calls Status.DeepCopyInto; HttpBinStatus.DeepCopyInto deep-copies Conditions slice and each metav1.Condition element.
Controller: HttpBin
internal/controller/httpbin_controller.go
Import k8s.io/apimachinery/pkg/api/meta; add setHttpBinConditionStatusCondition(...) helper using meta.SetStatusCondition; set/update httpBin.Status.Conditions on create errors and during status sync; include Conditions in status update logging.
Controller: HttpBinDeployment
internal/controller/httpbindeployment_controller.go
Import k8s.io/apimachinery/pkg/api/meta; add setHttpBinDeploymentStatusCondition(...) helper and call it across reconciliation (deployment/service/ingress create/update failures and successes, URL/endpoint readiness, readiness transitions); log Conditions in status updates.
CRD manifests: HttpBin CRD schema
charts/example-httpbin-operator/crds/..._httpbins.yaml, config/crd/bases/..._httpbins.yaml
Bump controller-gen/kubebuilder annotation (v0.18.0 → v0.19.0); add status.conditions OpenAPI v3 schema (array of Condition objects with lastTransitionTime, message, observedGeneration, reason, status, type and required subfields).
CRD manifests: HttpBinDeployment CRD annotations
charts/example-httpbin-operator/crds/..._httpbindeployments.yaml, config/crd/bases/..._httpbindeployments.yaml
Bump controller-gen/kubebuilder annotation (v0.18.0 → v0.19.0); no schema changes.
Build/tooling
go.mod
Update tooling: replace kubebuilder v3 tool entry with kubebuilder/v4 tooling and add/update sigs.k8s.io/kubebuilder/v4 v4.9.0 indirect requirement.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes


📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Free

📥 Commits

Reviewing files that changed from the base of the PR and between 8d58523 and 24a1a6b.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (10)
  • api/v1alpha1/httpbin_types.go (2 hunks)
  • api/v1alpha1/httpbindeployment_types.go (1 hunks)
  • api/v1alpha1/zz_generated.deepcopy.go (2 hunks)
  • charts/example-httpbin-operator/crds/orchestrate.platform-mesh.io_httpbindeployments.yaml (1 hunks)
  • charts/example-httpbin-operator/crds/orchestrate.platform-mesh.io_httpbins.yaml (2 hunks)
  • config/crd/bases/orchestrate.platform-mesh.io_httpbindeployments.yaml (1 hunks)
  • config/crd/bases/orchestrate.platform-mesh.io_httpbins.yaml (2 hunks)
  • go.mod (1 hunks)
  • internal/controller/httpbin_controller.go (3 hunks)
  • internal/controller/httpbindeployment_controller.go (10 hunks)

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

On-behalf-of: @SAP karol.szwaj@sap.com
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

On-behalf-of: @SAP karol.szwaj@sap.com
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

On-behalf-of: @SAP karol.szwaj@sap.com
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

On-behalf-of: @SAP karol.szwaj@sap.com
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds standardized Kubernetes conditions to the HttpBin and HttpBinDeployment custom resource status fields, enabling better observability of resource states. The changes provide clearer feedback on deployment readiness and failure scenarios through structured status conditions.

  • Introduces status conditions and reasons for both HttpBin and HttpBinDeployment resources
  • Updates controllers to set appropriate conditions during create, ready, and failure states
  • Improves deep-copy handling for status fields to prevent unintended mutations

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
internal/controller/httpbindeployment_controller.go Adds condition updates throughout reconciliation logic for deployment, service, and ingress operations
internal/controller/httpbin_controller.go Simplifies status updates and adds condition management for HttpBin resources
go.mod Updates Kubebuilder tooling from v3 to v4
api/v1alpha1/zz_generated.deepcopy.go Adds proper deep-copy handling for Conditions slice in HttpBinStatus
api/v1alpha1/httpbindeployment_types.go Defines condition types and reason constants for HttpBinDeployment
api/v1alpha1/httpbin_types.go Defines condition types and reason constants for HttpBin

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread internal/controller/httpbindeployment_controller.go Outdated
Comment thread internal/controller/httpbindeployment_controller.go Outdated
Comment thread internal/controller/httpbindeployment_controller.go Outdated
Comment thread internal/controller/httpbindeployment_controller.go Outdated
Comment thread internal/controller/httpbindeployment_controller.go Outdated
Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>

On-behalf-of: @SAP karol.szwaj@sap.com
@cnvergence
Copy link
Copy Markdown
Collaborator Author

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 26, 2025

✅ Actions performed

Full review triggered.

Copy link
Copy Markdown
Contributor

@mirzakopic mirzakopic left a comment

Choose a reason for hiding this comment

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

@cnvergence Could you please try to get make kind-test to work locally? I use that to quickly checkout the latest code and test it.

I would appreciate that if we have that in the repo, it makes local debugging and testing easier :)

@cnvergence
Copy link
Copy Markdown
Collaborator Author

it is a part of #25

@mirzakopic mirzakopic self-requested a review September 29, 2025 10:25
Copy link
Copy Markdown
Contributor

@mirzakopic mirzakopic left a comment

Choose a reason for hiding this comment

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

lgtm then, lets fix the make kind test as part of e2e issue

@cnvergence cnvergence merged commit 05a5756 into main Sep 29, 2025
12 checks passed
@cnvergence cnvergence deleted the conditions-region-feat branch October 2, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants