Skip to content

Commit

Permalink
UPSTREAM: <carry>: Reject the pod creation when we can not decide the…
Browse files Browse the repository at this point in the history
… cluster type

It is possible a race condition between pod creation and the update of the
infrastructure resource status with correct values under
Status.ControlPlaneTopology and Status.InfrastructureTopology.

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
  • Loading branch information
Artyom Lukianov authored and openshift-cherrypick-robot committed Jun 29, 2021
1 parent 766a5fe commit 0486e3f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
Expand Up @@ -180,6 +180,11 @@ func (a *managementCPUsOverride) Admit(ctx context.Context, attr admission.Attri
return admission.NewForbidden(attr, err) // can happen due to informer latency
}

// we can not decide the cluster type without status.controlPlaneTopology and status.infrastructureTopology
if clusterInfra.Status.ControlPlaneTopology == "" || clusterInfra.Status.InfrastructureTopology == "" {
return admission.NewForbidden(attr, fmt.Errorf("%s infrastructure resource has empty status.controlPlaneTopology or status.infrastructureTopology", PluginName))
}

// not the SNO cluster, skip mutation
// TODO: currently we supports only SNO use case because we have not yet worked out the best approach to determining whether the feature
// should be on or off in a multi-node cluster, and computing that state incorrectly could lead to breaking running clusters.
Expand Down
Expand Up @@ -168,6 +168,24 @@ func TestAdmit(t *testing.T) {
infra: testClusterSNOInfra(),
expectedError: fmt.Errorf(`failed to get workload annotation effect: the workload annotation value map["test":"test"] does not have "effect" key`),
},
{
name: "should return admission error when the infrastructure resource status has empty ControlPlaneTopology",
pod: testManagedPod("", "250m", "500Mi", "250Mi"),
expectedCpuRequest: resource.MustParse("250m"),
namespace: testManagedNamespace(),
nodes: []*corev1.Node{testNodeWithManagementResource()},
infra: testClusterInfraWithoutControlPlaneTopology(),
expectedError: fmt.Errorf("%s infrastructure resource has empty status.controlPlaneTopology or status.infrastructureTopology", PluginName),
},
{
name: "should return admission error when the infrastructure resource status has empty InfrastructureTopology",
pod: testManagedPod("", "250m", "500Mi", "250Mi"),
expectedCpuRequest: resource.MustParse("250m"),
namespace: testManagedNamespace(),
nodes: []*corev1.Node{testNodeWithManagementResource()},
infra: testClusterInfraWithoutInfrastructureTopology(),
expectedError: fmt.Errorf("%s infrastructure resource has empty status.controlPlaneTopology or status.infrastructureTopology", PluginName),
},
{
name: "should delete CPU requests and update workload CPU annotations for the burstable pod with managed annotation",
pod: testManagedPod("", "250m", "500Mi", "250Mi"),
Expand Down Expand Up @@ -687,3 +705,15 @@ func testClusterSNOInfra() *configv1.Infrastructure {
},
}
}

func testClusterInfraWithoutControlPlaneTopology() *configv1.Infrastructure {
infra := testClusterSNOInfra()
infra.Status.ControlPlaneTopology = ""
return infra
}

func testClusterInfraWithoutInfrastructureTopology() *configv1.Infrastructure {
infra := testClusterSNOInfra()
infra.Status.InfrastructureTopology = ""
return infra
}

0 comments on commit 0486e3f

Please sign in to comment.