Skip to content
Open
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
5 changes: 5 additions & 0 deletions internal/admission/admit_nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ func mutateNodePoolServiceProviderProperties(ctx context.Context, admissionConte
errs = append(errs, mutateNodePoolExperimentalTags(ctx, admissionContext, op)...)
errs = append(errs, mutateNodePoolCreateOperationCompletionDeadline(ctx, admissionContext, op, fldPath.Child("createOperationCompletionDeadline"), &newObj.CreateOperationCompletionDeadline)...)

if op.Type == operation.Create {
subscription := admissionContext.Subscription
newObj.ExperimentalFeaturesEnabled = subscription != nil && subscription.HasRegisteredFeature(metadataapi.FeatureExperimentalReleaseFeatures)
}

return errs
}

Expand Down
62 changes: 62 additions & 0 deletions internal/admission/admit_nodepool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,68 @@ func TestMutateNodePoolCreateOperationCompletionDeadline(t *testing.T) {
}
}

func TestMutateNodePoolExperimentalFeaturesEnabled(t *testing.T) {
afecRegistered := &coreapi.Subscription{
Properties: &coreapi.SubscriptionProperties{
RegisteredFeatures: &[]coreapi.Feature{
{
Name: ptr.To(metadataapi.FeatureExperimentalReleaseFeatures),
State: ptr.To("Registered"),
},
},
},
}

tests := []struct {
name string
op operation.Type
subscription *coreapi.Subscription
expected bool
}{
{
name: "create: AFEC registered sets ExperimentalFeaturesEnabled=true",
op: operation.Create,
subscription: afecRegistered,
expected: true,
},
{
name: "create: no AFEC leaves ExperimentalFeaturesEnabled=false",
op: operation.Create,
subscription: &coreapi.Subscription{Properties: &coreapi.SubscriptionProperties{}},
expected: false,
},
{
name: "create: nil subscription leaves ExperimentalFeaturesEnabled=false",
op: operation.Create,
subscription: nil,
expected: false,
},
{
name: "update: AFEC registered does not set ExperimentalFeaturesEnabled",
op: operation.Update,
subscription: afecRegistered,
expected: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
nodePool := &coreapi.HCPOpenShiftClusterNodePool{}
admissionContext := &NodePoolAdmissionContext{
Clock: utilsclock.RealClock{},
Subscription: tt.subscription,
Cluster: &coreapi.HCPOpenShiftCluster{},
}
if tt.op == operation.Update {
admissionContext.OriginalNodePool = &coreapi.HCPOpenShiftClusterNodePool{}
}
errs := MutateNodePool(context.Background(), admissionContext, operation.Operation{Type: tt.op}, nodePool, admissionContext.OriginalNodePool)
require.Empty(t, errs)
assert.Equal(t, tt.expected, nodePool.ServiceProviderProperties.ExperimentalFeaturesEnabled)
})
}
}

func TestAdmitNodePool_SubnetVNet(t *testing.T) {
const (
clusterSubnet = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/cluster-vnet/subnets/cluster-subnet"
Expand Down
7 changes: 7 additions & 0 deletions internal/api/coreapi/types_nodepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ type HCPOpenShiftClusterNodePoolServiceProviderProperties struct {
// The operation node pool create controller uses this value to decide about marking the install as failed.
// The e2e tests set this value to one minute less than the default timeout.
CreateOperationCompletionDeadline *metav1.Time `json:"createOperationCompletionDeadline,omitempty"`

// ExperimentalFeaturesEnabled records whether the FeatureExperimentalReleaseFeatures AFEC was
// registered on the subscription at node pool creation time. Used by internal/ocm/convert.go
// to gate experimental CS fields (e.g. SseEncryptionSetResourceId on the OS disk) that are
// only sent to Cluster Service when the AFEC is registered.
// Written by: Frontend PUT NodePool (Create)
ExperimentalFeaturesEnabled bool `json:"experimentalFeaturesEnabled,omitempty"`
Comment on lines +128 to +133
}

// NodePoolVersionProfile represents the worker node pool version.
Expand Down
1 change: 1 addition & 0 deletions internal/apitesting/coreapitesting/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ func CommonRoundTripFuzzFuncs() []interface{} {
j.ActiveOperationID = ""
j.ClusterServiceID = nil
j.UsesNewNodePoolDeletionApproach = false
j.ExperimentalFeaturesEnabled = false
},
func(j *coreapi.HCPOpenShiftClusterExternalAuthServiceProviderProperties, c randfill.Continue) {
c.FillNoCustom(j)
Expand Down
16 changes: 12 additions & 4 deletions internal/ocm/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,17 @@ func convertEnableEncryptionAtHostToCSBuilder(in coreapi.NodePoolPlatformProfile
return arohcpv1alpha1.NewAzureNodePoolEncryptionAtHost().State(state)
}

func buildCSOsDisk(osDisk coreapi.OSDiskProfile, storageAccountType, persistence string, experimentalFeaturesEnabled bool) *arohcpv1alpha1.AzureNodePoolOsDiskBuilder {
builder := arohcpv1alpha1.NewAzureNodePoolOsDisk().
SizeGibibytes(int(*osDisk.SizeGiB)).
StorageAccountType(storageAccountType).
Persistence(persistence)
if experimentalFeaturesEnabled && osDisk.EncryptionSetID != nil {

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.

n.b. the field is ignored today, which is why it's correct to do this

builder.SseEncryptionSetResourceId(osDisk.EncryptionSetID.String())
}
return builder
}

func convertClusterImageRegistryStateRPToCS(in coreapi.ClusterImageRegistryProfile) (string, error) {
switch in.State {
case metadataapi.ClusterImageRegistryStateDisabled:
Expand Down Expand Up @@ -629,10 +640,7 @@ func BuildCSNodePool(ctx context.Context, nodePool *coreapi.HCPOpenShiftClusterN
ResourceName(strings.ToLower(nodePool.Name)).
VMSize(nodePool.Properties.Platform.VMSize).
EncryptionAtHost(convertEnableEncryptionAtHostToCSBuilder(nodePool.Properties.Platform)).
OsDisk(arohcpv1alpha1.NewAzureNodePoolOsDisk().
SizeGibibytes(int(*nodePool.Properties.Platform.OSDisk.SizeGiB)).
StorageAccountType(csDiskStorageAccountType).
Persistence(csPersistence))).
OsDisk(buildCSOsDisk(nodePool.Properties.Platform.OSDisk, csDiskStorageAccountType, csPersistence, nodePool.ServiceProviderProperties.ExperimentalFeaturesEnabled))).
AvailabilityZone(nodePool.Properties.Platform.AvailabilityZone).
AutoRepair(nodePool.Properties.AutoRepair)
}
Expand Down
40 changes: 40 additions & 0 deletions internal/ocm/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,46 @@ func TestBuildCSNodePool(t *testing.T) {
),
),
},
{
name: "passes disk encryption set ID to CS when AFEC is enabled",
hcpNodePool: getHCPNodePoolResource(
func(hsc *coreapi.HCPOpenShiftClusterNodePool) {
hsc.Properties.Platform.OSDisk.EncryptionSetID = metadataapi.Must(azcorearm.ParseResourceID(
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.Compute/diskEncryptionSets/test-des"))
hsc.ServiceProviderProperties.ExperimentalFeaturesEnabled = true
},
),
expectedCSNodePool: getBaseCSNodePoolBuilder().
AzureNodePool(arohcpv1alpha1.NewAzureNodePool().
ResourceName("").
VMSize("").
EncryptionAtHost(
arohcpv1alpha1.NewAzureNodePoolEncryptionAtHost().
State(csEncryptionAtHostStateDisabled),
).
OsDisk(arohcpv1alpha1.NewAzureNodePoolOsDisk().
SizeGibibytes(64).
StorageAccountType(string(metadataapi.DiskStorageAccountTypePremium_LRS)).
Persistence("persistent").
SseEncryptionSetResourceId("/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.Compute/diskEncryptionSets/test-des"),
),
),
},
{
name: "disk encryption set ID not passed to CS when AFEC is disabled",
hcpNodePool: getHCPNodePoolResource(
func(hsc *coreapi.HCPOpenShiftClusterNodePool) {
hsc.Properties.Platform.OSDisk.EncryptionSetID = metadataapi.Must(azcorearm.ParseResourceID(
"/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/test-rg/providers/Microsoft.Compute/diskEncryptionSets/test-des"))
},
),
expectedCSNodePool: getBaseCSNodePoolBuilder(),
},
{
name: "nil disk encryption set ID does not set SSE field",
hcpNodePool: getHCPNodePoolResource(),
expectedCSNodePool: getBaseCSNodePoolBuilder(),
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down