Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 5 additions & 2 deletions admin/server/handlers/hcp/breakglass/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,15 @@ func (h *HCPBreakglassSessionCreationHandler) ServeHTTP(writer http.ResponseWrit
return fmt.Errorf("failed to get HCP from database: %w", err)
}

clusterHypershiftDetails, err := h.csClient.GetClusterHypershiftDetails(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID)
if hcp.ServiceProviderProperties.ClusterServiceID == nil {
return fmt.Errorf("cluster has no ClusterServiceID")
}
clusterHypershiftDetails, err := h.csClient.GetClusterHypershiftDetails(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return hcphelpers.ClusterServiceError(err, "hypershift details")
}

provisionShard, err := h.csClient.GetClusterProvisionShard(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID)
provisionShard, err := h.csClient.GetClusterProvisionShard(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return hcphelpers.ClusterServiceError(err, "provision shard")
}
Expand Down
5 changes: 4 additions & 1 deletion admin/server/handlers/hcp/helloworld.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ func (h *HCPHelloWorldHandler) ServeHTTP(writer http.ResponseWriter, request *ht
}

// get CS cluster data - once the sync from CS to cosmos is in place, we should not need this anymore
csCluster, err := h.csClient.GetCluster(request.Context(), hcp.ServiceProviderProperties.ClusterServiceID)
if hcp.ServiceProviderProperties.ClusterServiceID == nil {
return fmt.Errorf("cluster has no ClusterServiceID")
}
csCluster, err := h.csClient.GetCluster(request.Context(), *hcp.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return fmt.Errorf("failed to get CS cluster data: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions admin/server/handlers/hcp/serialconsole_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func TestSerialConsoleHandler(t *testing.T) {
Resource: arm.Resource{ID: resourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: internalID,
ClusterServiceID: &internalID,
},
}
_, err = mockDB.HCPClusters(resourceID.SubscriptionID, resourceID.ResourceGroupName).Create(ctx, hcp, nil)
Expand All @@ -121,7 +121,7 @@ func TestSerialConsoleHandler(t *testing.T) {
Resource: arm.Resource{ID: resourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: internalID,
ClusterServiceID: &internalID,
},
}
_, err = mockDB.HCPClusters(resourceID.SubscriptionID, resourceID.ResourceGroupName).Create(ctx, hcp, nil)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func newTestCluster(t *testing.T, clusterUID string, provisioningState arm.Provi
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ProvisioningState: provisioningState,
ClusterUID: clusterUID,
ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))),
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestOrphanedBillingCleanup_SyncOnce(t *testing.T) {
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ProvisioningState: arm.ProvisioningStateSucceeded,
ClusterUID: "billing-doc-2",
ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))),
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (c *clusterCustomerPropertiesMigrationController) CooldownChecker() control
func (c *clusterCustomerPropertiesMigrationController) NeedsWork(ctx context.Context, existingCluster *api.HCPOpenShiftCluster) bool {
// Check if we have a cluster service ID to query. We will lack this information for newly created records when we
// transition to async cluster-service creation.
if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
return false
}

Expand Down Expand Up @@ -124,7 +124,7 @@ func (c *clusterCustomerPropertiesMigrationController) SyncOnce(ctx context.Cont
}

// Fetch the cluster from Cluster Service
csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID)
csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (c *clusterPropertiesSyncer) SyncOnce(ctx context.Context, key controllerut
}

// Check if we have a cluster service ID to query
if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
return nil
}

Expand All @@ -112,7 +112,7 @@ func (c *clusterPropertiesSyncer) SyncOnce(ctx context.Context, key controllerut
}

// Fetch the cluster from Cluster Service
csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID)
csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ func newTestCluster(opts ...func(*api.HCPOpenShiftCluster)) *api.HCPOpenShiftClu
},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID(testClusterServiceIDStr)),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID(testClusterServiceIDStr))),
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (c *identityMigrationSyncer) CooldownChecker() controllerutils.CooldownChec

func (c *identityMigrationSyncer) NeedsWork(ctx context.Context, existingCluster *api.HCPOpenShiftCluster) bool {
// Check if we have a cluster service ID to query
if len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
if existingCluster.ServiceProviderProperties.ClusterServiceID == nil || len(existingCluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
return false
}

Expand Down Expand Up @@ -134,7 +134,7 @@ func (c *identityMigrationSyncer) SyncOnce(ctx context.Context, key controllerut
}

// Fetch the cluster from Cluster Service
csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID)
csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get cluster from Cluster Service: %w", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,11 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get Cluster: %w", err))
}
if existingCluster.ServiceProviderProperties.ClusterServiceID == nil {
// we don't have enough information to proceed. We will retrigger once the information is present.
// TODO remove this once we have the information all in cosmos.
return nil
}

existingServiceProviderCluster, err := database.GetOrCreateServiceProviderCluster(ctx, c.cosmosClient, key.GetResourceID())
if err != nil {
Expand Down Expand Up @@ -156,7 +161,7 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C
// we are guaranteed to have a shard allocated for the cluster. If this changes in the future
// we would need to change the logic in controllers to check that the retrieved cluster has a
// shard allocated.
clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID)
clusterProvisionShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID)
Comment thread
miguelsorianod marked this conversation as resolved.
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get Cluster Provision Shard from Cluster Service: %w", err))
}
Expand All @@ -170,7 +175,7 @@ func (c *createClusterScopedMaestroReadonlyBundlesSyncer) SyncOnce(ctx context.C
return utils.TrackError(fmt.Errorf("failed to create Maestro client: %w", err))
}

csCluster, err := c.clusterServiceClient.GetCluster(ctx, existingCluster.ServiceProviderProperties.ClusterServiceID)
csCluster, err := c.clusterServiceClient.GetCluster(ctx, *existingCluster.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return utils.TrackError(fmt.Errorf("failed to get Cluster from Cluster Service: %w", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func TestBuildInitialReadonlyMaestroBundleForHostedCluster(t *testing.T) {
},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}

Expand Down Expand Up @@ -409,7 +409,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_syncMaestroBundle(t *te
Resource: arm.Resource{ID: clusterResourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}

Expand Down Expand Up @@ -545,7 +545,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_GetServiceProv
cluster := &api.HCPOpenShiftCluster{
TrackedResource: arm.TrackedResource{Resource: arm.Resource{ID: clusterResourceID}},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}

Expand Down Expand Up @@ -597,7 +597,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_AllBundlesAlre
Resource: arm.Resource{ID: clusterResourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}
clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName)
Expand Down Expand Up @@ -669,7 +669,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecut
},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}

Expand Down Expand Up @@ -698,15 +698,15 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_SyncLoopExecut
// Setup cluster service mocks
provisionShard := buildTestProvisionShard("test-consumer")
mockClusterService.EXPECT().
GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).
GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).
Return(provisionShard, nil)

csCluster, err := arohcpv1alpha1.NewCluster().
DomainPrefix("test-domain").
Build()
require.NoError(t, err)
mockClusterService.EXPECT().
GetCluster(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).
GetCluster(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).
Return(csCluster, nil)

// Setup maestro builder mock
Expand Down Expand Up @@ -781,7 +781,7 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_ProcessesParti
Resource: arm.Resource{ID: clusterResourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111")),
ClusterServiceID: api.Ptr(api.Must(api.NewInternalID("/api/aro_hcp/v1alpha1/clusters/11111111111111111111111111111111"))),
},
}
clustersCRUD := mockDBClient.HCPClusters(key.SubscriptionID, key.ResourceGroupName)
Expand Down Expand Up @@ -815,12 +815,12 @@ func TestCreateClusterScopedMaestroReadonlyBundlesSyncer_SyncOnce_ProcessesParti

provisionShard := buildTestProvisionShard("test-consumer")
mockClusterService.EXPECT().
GetClusterProvisionShard(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).
GetClusterProvisionShard(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).
Return(provisionShard, nil)
csCluster, err := arohcpv1alpha1.NewCluster().DomainPrefix("test-domain").Build()
require.NoError(t, err)
mockClusterService.EXPECT().
GetCluster(gomock.Any(), cluster.ServiceProviderProperties.ClusterServiceID).
GetCluster(gomock.Any(), *cluster.ServiceProviderProperties.ClusterServiceID).
Return(csCluster, nil)

restEndpoint := provisionShard.MaestroConfig().RestApiConfig().Url()
Expand Down
4 changes: 2 additions & 2 deletions backend/pkg/controllers/datadumpcontrollers/cs_state_dump.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ func (c *csStateDump) SyncOnce(ctx context.Context, key controllerutils.HCPClust
return nil // best effort, don't fail
}

csID := cluster.ServiceProviderProperties.ClusterServiceID
if len(csID.String()) == 0 {
if cluster.ServiceProviderProperties.ClusterServiceID == nil || len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
// No ClusterServiceID yet, cluster hasn't been registered with CS
return nil
}
csID := *cluster.ServiceProviderProperties.ClusterServiceID

// Fetch cluster state from cluster-service
csCluster, err := c.csClient.GetCluster(ctx, csID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func TestCSStateDump_SyncOnce(t *testing.T) {
Resource: arm.Resource{ID: clusterResourceID},
},
ServiceProviderProperties: api.HCPOpenShiftClusterServiceProviderProperties{
ClusterServiceID: csID,
ClusterServiceID: &csID,
},
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,15 +478,15 @@ func (c *deleteOrphanedMaestroReadonlyBundles) clusterProvisionShardIDForService
// provisionShardIDFromCluster resolves the provision shard for a Cosmos cluster document. skip is true when ClusterServiceID
// is unset so the cluster is not yet registered with Cluster Service (same gate as create-*-scoped Maestro bundle controllers).
func (c *deleteOrphanedMaestroReadonlyBundles) provisionShardIDFromCluster(ctx context.Context, cluster *api.HCPOpenShiftCluster) (shardID string, skip bool, err error) {
if len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
if cluster.ServiceProviderProperties.ClusterServiceID == nil || len(cluster.ServiceProviderProperties.ClusterServiceID.String()) == 0 {
return "", true, nil
}
// TODO We get the provision shard ID from CS but at some point we should have
// the information in Cosmos and this should be changed to use that instead.
// TODO should we take into account that at some point in the future we will implement migration between management
// clusters, where a cluster could have bundles allocated to different provision shards at the same time? For now
// we assume that the cluster is associated to a single provision shard at a time.
clusterCSShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, cluster.ServiceProviderProperties.ClusterServiceID)
clusterCSShard, err := c.clusterServiceClient.GetClusterProvisionShard(ctx, *cluster.ServiceProviderProperties.ClusterServiceID)
if err != nil {
return "", false, utils.TrackError(fmt.Errorf("failed to get Cluster Provision Shard: %w", err))
}
Expand Down
Loading
Loading