Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

feat: configurable disk caching #2863

Merged
merged 5 commits into from
Apr 30, 2020
Merged

feat: configurable disk caching #2863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
01ffaa0
feat: configurable disk caching
jackfrancis Mar 9, 2020
a5dda33
actually implement
jackfrancis Apr 28, 2020
e23879e
more UT updates
jackfrancis Apr 28, 2020
9d49220
test: update validate UT
jackfrancis Apr 30, 2020
89f1739
test: update test configs
jackfrancis Apr 30, 2020
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
3 changes: 3 additions & 0 deletions docs/topics/clusterdefinitions.md
View file
Open in desktop

Large diffs are not rendered by default.

7 changes: 5 additions & 2 deletions examples/e2e-tests/kubernetes/release/default/definition.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@
128
],
"availabilityProfile": "VirtualMachineScaleSets",
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME"
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
"osDiskCachingType": "ReadOnly",
"dataDiskCachingType": "ReadWrite"
},
{
"name": "poolwin",
Expand Down Expand Up @@ -121,7 +123,8 @@
"vmSize": "Standard_D2s_v3",
"osType": "Windows",
"availabilityProfile": "VirtualMachineScaleSets",
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME"
"vnetSubnetId": "/subscriptions/SUB_ID/resourceGroups/RG_NAME/providers/Microsoft.Network/virtualNetworks/VNET_NAME/subnets/SUBNET_NAME",
"osDiskCachingType": "ReadOnly"
Copy link
Member Author

Choose a reason for hiding this comment

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

@marosset FYI, adding this new configuration to a Windows pool in our E2E cluster configuration to confirm that this is not a Linux/Windows thing.

Btw, should we add a data disk to this pool to (1) test that functionality generally, and to also validate the data disk caching type override?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure.

}
],
"linuxProfile": {
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/converterfromapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,7 @@ func convertMasterProfileToVLabs(api *MasterProfile, vlabsProfile *vlabs.MasterP
vlabsProfile.UltraSSDEnabled = api.UltraSSDEnabled
vlabsProfile.EncryptionAtHost = api.EncryptionAtHost
vlabsProfile.ProximityPlacementGroupID = api.ProximityPlacementGroupID
vlabsProfile.OSDiskCachingType = api.OSDiskCachingType
convertCustomFilesToVlabs(api, vlabsProfile)
vlabsProfile.SysctlDConfig = map[string]string{}
for key, val := range api.SysctlDConfig {
Expand Down Expand Up @@ -624,6 +625,8 @@ func convertAgentPoolProfileToVLabs(api *AgentPoolProfile, p *vlabs.AgentPoolPro
for key, val := range api.SysctlDConfig {
p.SysctlDConfig[key] = val
}
p.OSDiskCachingType = api.OSDiskCachingType
p.DataDiskCachingType = api.DataDiskCachingType
}

func convertServicePrincipalProfileToVLabs(api *ServicePrincipalProfile, v *vlabs.ServicePrincipalProfile) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/convertertoapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ func convertVLabsMasterProfile(vlabs *vlabs.MasterProfile, api *MasterProfile) {
api.EncryptionAtHost = vlabs.EncryptionAtHost
api.AuditDEnabled = vlabs.AuditDEnabled
api.ProximityPlacementGroupID = vlabs.ProximityPlacementGroupID
api.OSDiskCachingType = vlabs.OSDiskCachingType
convertCustomFilesToAPI(vlabs, api)
api.SysctlDConfig = map[string]string{}
for key, val := range vlabs.SysctlDConfig {
Expand Down Expand Up @@ -671,6 +672,8 @@ func convertVLabsAgentPoolProfile(vlabs *vlabs.AgentPoolProfile, api *AgentPoolP
for key, val := range vlabs.SysctlDConfig {
api.SysctlDConfig[key] = val
}
api.OSDiskCachingType = vlabs.OSDiskCachingType
api.DataDiskCachingType = vlabs.DataDiskCachingType
}

func convertVLabsKeyVaultSecrets(vlabs *vlabs.KeyVaultSecrets, api *KeyVaultSecrets) {
Expand Down
16 changes: 16 additions & 0 deletions pkg/api/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"strconv"
"strings"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
"github.com/Azure/go-autorest/autorest/to"

"github.com/blang/semver"
Expand Down Expand Up @@ -691,6 +692,10 @@ func (p *Properties) setMasterProfileDefaults(isUpgrade bool) {
if p.MasterProfile.PlatformUpdateDomainCount == nil {
p.MasterProfile.PlatformUpdateDomainCount = to.IntPtr(3)
}

if p.MasterProfile.OSDiskCachingType == "" {
p.MasterProfile.OSDiskCachingType = string(compute.CachingTypesReadWrite)
}
}

func (p *Properties) setAgentProfileDefaults(isUpgrade, isScale bool) {
Expand Down Expand Up @@ -761,6 +766,17 @@ func (p *Properties) setAgentProfileDefaults(isUpgrade, isScale bool) {
if !p.OrchestratorProfile.IsKubernetes() {
profile.Distro = Ubuntu
}

if profile.OSDiskCachingType == "" {
if profile.IsEphemeral() {
profile.OSDiskCachingType = string(compute.CachingTypesReadOnly)
} else {
profile.OSDiskCachingType = string(compute.CachingTypesReadWrite)
}
}
if profile.DataDiskCachingType == "" {
profile.DataDiskCachingType = string(compute.CachingTypesReadOnly)
}
}
}

Expand Down
28 changes: 28 additions & 0 deletions pkg/api/defaults_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"testing"

"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
"github.com/Azure/go-autorest/autorest/azure"
"github.com/Azure/go-autorest/autorest/to"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -737,6 +738,29 @@ func TestAuditDEnabled(t *testing.T) {
}
}

func TestDiskCachingTypes(t *testing.T) {
mockCS := getMockBaseContainerService("1.18.2")
mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes
isUpgrade := false
isScale := false
mockCS.Properties.setAgentProfileDefaults(isUpgrade, isScale)

if mockCS.Properties.AgentPoolProfiles[0].OSDiskCachingType != string(compute.CachingTypesReadWrite) {
t.Errorf("expected OSDiskCachingType to be %s by default, instead got %s", string(compute.CachingTypesReadWrite), mockCS.Properties.AgentPoolProfiles[0].OSDiskCachingType)
}
if mockCS.Properties.AgentPoolProfiles[0].DataDiskCachingType != string(compute.CachingTypesReadOnly) {
t.Errorf("expected OSDiskCachingType to be %s by default, instead got %s", string(compute.CachingTypesReadOnly), mockCS.Properties.AgentPoolProfiles[0].OSDiskCachingType)
}

mockCS = getMockBaseContainerService("1.18.2")
mockCS.Properties.OrchestratorProfile.OrchestratorType = Kubernetes
mockCS.Properties.AgentPoolProfiles[0].StorageProfile = Ephemeral
mockCS.Properties.setAgentProfileDefaults(isUpgrade, isScale)
if mockCS.Properties.AgentPoolProfiles[0].OSDiskCachingType != string(compute.CachingTypesReadOnly) {
t.Errorf("expected OSDiskCachingType to be %s by default, instead got %s", string(compute.CachingTypesReadOnly), mockCS.Properties.AgentPoolProfiles[0].OSDiskCachingType)
}
}

func TestKubeletFeatureGatesEnsureFeatureGatesOnAgentsFor1_6_0(t *testing.T) {
mockCS := getMockBaseContainerService("1.6.0")
properties := mockCS.Properties
Expand Down Expand Up @@ -1402,6 +1426,10 @@ func TestMasterProfileDefaults(t *testing.T) {
t.Fatalf("Master VMAS, AzureCNI: MasterProfile FirstConsecutiveStaticIP did not have the expected default configuration, got %s, expected %s",
properties.MasterProfile.FirstConsecutiveStaticIP, "10.255.255.5")
}
if properties.MasterProfile.OSDiskCachingType != string(compute.CachingTypesReadWrite) {
t.Fatalf("MasterProfile.OSDiskCachingType did not have the expected default configuration, got %s, expected %s",
properties.MasterProfile.OSDiskCachingType, string(compute.CachingTypesReadWrite))
}

// this validates default VMSS masterProfile configuration
mockCS = getMockBaseContainerService("1.13.12")
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,7 @@ type MasterProfile struct {
CosmosEtcd *bool `json:"cosmosEtcd,omitempty"`
SysctlDConfig map[string]string `json:"sysctldConfig,omitempty"`
ProximityPlacementGroupID string `json:"proximityPlacementGroupID,omitempty"`
OSDiskCachingType string `json:"osDiskCachingType,omitempty"`
}

// ImageReference represents a reference to an Image resource in Azure.
Expand Down Expand Up @@ -664,6 +665,8 @@ type AgentPoolProfile struct {
UltraSSDEnabled *bool `json:"ultraSSDEnabled,omitempty"`
EncryptionAtHost *bool `json:"encryptionAtHost,omitempty"`
ProximityPlacementGroupID string `json:"proximityPlacementGroupID,omitempty"`
OSDiskCachingType string `json:"osDiskCachingType,omitempty"`
DataDiskCachingType string `json:"dataDiskCachingType,omitempty"`
}

// AgentPoolProfileRole represents an agent role
Expand Down
3 changes: 3 additions & 0 deletions pkg/api/vlabs/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ type MasterProfile struct {
// True: uses cosmos etcd endpoint instead of installing etcd on masters
CosmosEtcd *bool `json:"cosmosEtcd,omitempty"`
ProximityPlacementGroupID string `json:"proximityPlacementGroupID,omitempty"`
OSDiskCachingType string `json:"osDiskCachingType,omitempty"`
}

// ImageReference represents a reference to an Image resource in Azure.
Expand Down Expand Up @@ -529,6 +530,8 @@ type AgentPoolProfile struct {
LoadBalancerBackendAddressPoolIDs []string `json:"loadBalancerBackendAddressPoolIDs,omitempty"`
SysctlDConfig map[string]string `json:"sysctldConfig,omitempty"`
ProximityPlacementGroupID string `json:"proximityPlacementGroupID,omitempty"`
OSDiskCachingType string `json:"osDiskCachingType,omitempty"`
DataDiskCachingType string `json:"dataDiskCachingType,omitempty"`
}

// AgentPoolProfileRole represents an agent role
Expand Down
32 changes: 32 additions & 0 deletions pkg/api/vlabs/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/Azure/aks-engine/pkg/api/common"
"github.com/Azure/aks-engine/pkg/helpers"
"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2019-12-01/compute"
"github.com/Azure/go-autorest/autorest/to"
"github.com/blang/semver"
"github.com/google/uuid"
Expand All @@ -38,6 +39,7 @@ var (
"3.2.13", "3.2.14", "3.2.15", "3.2.16", "3.2.23", "3.2.24", "3.2.25", "3.2.26", "3.3.0", "3.3.1", "3.3.8", "3.3.9", "3.3.10", "3.3.13", "3.3.15", "3.3.18", "3.3.19"}
containerdValidVersions = [...]string{"1.3.2"}
kubernetesImageBaseTypeValidVersions = [...]string{"", common.KubernetesImageBaseTypeGCR, common.KubernetesImageBaseTypeMCR}
cachingTypesValidValues = [...]string{"", string(compute.CachingTypesNone), string(compute.CachingTypesReadWrite), string(compute.CachingTypesReadOnly)}
networkPluginPlusPolicyAllowed = []k8sNetworkConfig{
{
networkPlugin: "",
Expand Down Expand Up @@ -456,6 +458,16 @@ func (a *Properties) validateMasterProfile(isUpdate bool) error {
}
}

var validOSDiskCachingType bool
for _, valid := range cachingTypesValidValues {
if valid == m.OSDiskCachingType {
validOSDiskCachingType = true
}
}
if !validOSDiskCachingType {
return errors.Errorf("Invalid masterProfile osDiskCachingType value \"%s\", please use one of the following versions: %s", m.OSDiskCachingType, cachingTypesValidValues)
}

return common.ValidateDNSPrefix(m.DNSPrefix)
}

Expand Down Expand Up @@ -579,6 +591,26 @@ func (a *Properties) validateAgentPoolProfiles(isUpdate bool) error {
if e := validateProximityPlacementGroupID(agentPoolProfile.ProximityPlacementGroupID); e != nil {
return e
}
var validOSDiskCachingType, validDataDiskCachingType bool
for _, valid := range cachingTypesValidValues {
if valid == agentPoolProfile.OSDiskCachingType {
validOSDiskCachingType = true
}
if valid == agentPoolProfile.DataDiskCachingType {
validDataDiskCachingType = true
}
}
if !validOSDiskCachingType {
return errors.Errorf("Invalid osDiskCachingType value \"%s\" for agentPoolProfile \"%s\", please use one of the following versions: %s", agentPoolProfile.OSDiskCachingType, agentPoolProfile.Name, cachingTypesValidValues)
}
if !validDataDiskCachingType {
return errors.Errorf("Invalid dataDiskCachingType value \"%s\" for agentPoolProfile \"%s\", please use one of the following versions: %s", agentPoolProfile.DataDiskCachingType, agentPoolProfile.Name, cachingTypesValidValues)
}
if agentPoolProfile.IsEphemeral() {
if agentPoolProfile.OSDiskCachingType != "" && agentPoolProfile.OSDiskCachingType != string(compute.CachingTypesReadOnly) {
return errors.Errorf("Invalid osDiskCachingType value \"%s\" for agentPoolProfile \"%s\" using Ephemeral Disk, you must use: %s", agentPoolProfile.OSDiskCachingType, agentPoolProfile.Name, string(compute.CachingTypesReadOnly))
}
}
}

return nil
Expand Down
Loading