Skip to content

interface: add status object for managed namespace placement#1164

Closed
britaniar wants to merge 5 commits into
Azure:mainfrom
britaniar:addAPIforManagedNamespace
Closed

interface: add status object for managed namespace placement#1164
britaniar wants to merge 5 commits into
Azure:mainfrom
britaniar:addAPIforManagedNamespace

Conversation

@britaniar

@britaniar britaniar commented Jul 30, 2025

Copy link
Copy Markdown
Contributor

Description of your changes

Fixes #

I have: created an object that CRP should add to the namespace that the user can access so they can determine which clusters the namespace has been placed on to.

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

@britaniar
britaniar requested a review from Copilot July 30, 2025 23:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 introduces a new status object for managed namespace placement functionality by adding a ManagedNamespacePlacement custom resource. The purpose is to track which clusters a namespace has been placed on, providing visibility into namespace placement status across the fleet.

Key changes include:

  • Addition of a new ManagedNamespacePlacement CRD with status tracking capabilities
  • Registration of the new type in the scheme builder

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
config/crd/bases/placement.kubernetes-fleet.io_managednamespaceplacements.yaml Adds complete CRD definition for ManagedNamespacePlacement with comprehensive status object structure
apis/placement/v1beta1/clusterresourceplacement_types.go Defines the Go struct for ManagedNamespacePlacement and registers it in the scheme

Comment thread apis/placement/v1beta1/clusterresourceplacement_types.go Outdated
Comment thread apis/placement/v1beta1/clusterresourceplacement_types.go Outdated
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
@britaniar
britaniar marked this pull request as ready for review July 30, 2025 23:42
Comment thread apis/placement/v1beta1/clusterresourceplacement_types.go
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// ManagedNamespacePlacement is used to represent which clusters the namespace has been placed on to.
type ManagedNamespacePlacement struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do you use managednamespace as a prefix? Should this object exist for namespaces placed by a CRP?

type ManagedNamespacePlacementStatus struct {
// ResourcePlacements contains the placement statuses for the namespace.
// +kubebuilder:validation:Optional
ResourcePlacements []ResourcePlacementStatus `json:"resourcePlacements,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does the status represent? Only the status of the namespace or namespace+resources as selected by the CRP?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume this is a copy of the ResourcePlacementStatus list from CRP. If so, I would recommend to copy the whole status as SelectedResources and ObservedResourceIndex are also critical information to understand the conditions.

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// ManagedNamespacePlacement is used to represent which clusters the namespace has been placed on to.
type ManagedNamespacePlacement struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How do you know which CRP this status is for? Can one namespace selected/placed by N CRPs?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this object won't exist when there are N CRPs, provide a comment.

@ryanzhang-oss

Copy link
Copy Markdown
Contributor

close this as we should move this to kubefleet. Will address comments there

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.

4 participants