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

Extracting property values to make ARM output variables accessible #3877

Merged
merged 17 commits into from
Sep 25, 2018
Merged
Show file tree
Hide file tree
Changes from 15 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
4 changes: 0 additions & 4 deletions pkg/acsengine/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,6 @@ const (
DefaultKubernetesGCLowThreshold = 80
// DefaultGeneratorCode specifies the source generator of the cluster template.
DefaultGeneratorCode = "acsengine"
// DefaultOrchestratorName specifies the 3 character orchestrator code of the cluster template and affects resource naming.
DefaultOrchestratorName = "k8s"
// DefaultOpenshiftOrchestratorName specifies the 3 character orchestrator code of the cluster template and affects resource naming.
DefaultOpenshiftOrchestratorName = "ocp"
// DefaultEtcdVersion specifies the default etcd version to install
DefaultEtcdVersion = "3.2.23"
// DefaultEtcdDiskSize specifies the default size for Kubernetes master etcd disk volumes in GB
Expand Down
2 changes: 1 addition & 1 deletion pkg/acsengine/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ func setHostedMasterProfileDefaults(a *api.Properties) {

func setDefaultCerts(a *api.Properties) (bool, error) {
if a.MasterProfile != nil && a.OrchestratorProfile.OrchestratorType == api.OpenShift {
return certgen.OpenShiftSetDefaultCerts(a, DefaultOpenshiftOrchestratorName, GenerateClusterID(a))
return certgen.OpenShiftSetDefaultCerts(a, api.DefaultOpenshiftOrchestratorName, a.GenerateClusterID())
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: GenerateClusterID seems to suggest to generate every time it is called... GetClusterID seems better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly!

Copy link
Contributor

Choose a reason for hiding this comment

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

ok.. maybe it's generated after all...

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify, the existing GenerateClusterID (before this change) also generates w/ every invocation as well, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Talked to @tariq1890 , it's seeded using the hash. so clusterID is always the same regardless how many times it is called

Copy link
Contributor

Choose a reason for hiding this comment

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

In this case, I'd like to get GetClusterID :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jackfrancis no, I've added a new field in Properties to store the ClusterID after it's generated during the first time.

}

if a.MasterProfile == nil || a.OrchestratorProfile.OrchestratorType != api.Kubernetes {
Expand Down
31 changes: 0 additions & 31 deletions pkg/acsengine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"hash/fnv"
"io/ioutil"
"log"
"math/rand"
"net"
"net/http"
"regexp"
Expand Down Expand Up @@ -46,23 +44,6 @@ func init() {
keyvaultSecretPathRe = regexp.MustCompile(`^(/subscriptions/\S+/resourceGroups/\S+/providers/Microsoft.KeyVault/vaults/\S+)/secrets/([^/\s]+)(/(\S+))?$`)
}

// GenerateClusterID creates a unique 8 string cluster ID
func GenerateClusterID(properties *api.Properties) string {
uniqueNameSuffixSize := 8
// the name suffix uniquely identifies the cluster and is generated off a hash
// from the master dns name
h := fnv.New64a()
if properties.MasterProfile != nil {
h.Write([]byte(properties.MasterProfile.DNSPrefix))
} else if properties.HostedMasterProfile != nil {
h.Write([]byte(properties.HostedMasterProfile.DNSPrefix))
} else {
h.Write([]byte(properties.AgentPoolProfiles[0].Name))
}
rand.Seed(int64(h.Sum64()))
return fmt.Sprintf("%08d", rand.Uint32())[:uniqueNameSuffixSize]
}

// GenerateKubeConfig returns a JSON string representing the KubeConfig
func GenerateKubeConfig(properties *api.Properties, location string) (string, error) {
if properties == nil {
Expand Down Expand Up @@ -478,18 +459,6 @@ func isNSeriesSKU(profile *api.AgentPoolProfile) bool {
return false
}

func isCustomVNET(a []*api.AgentPoolProfile) bool {
if a != nil {
for _, agentPoolProfile := range a {
if !agentPoolProfile.IsCustomVNET() {
return false
}
}
return true
}
return false
}

func getDCOSCustomDataPublicIPStr(orchestratorType string, masterCount int) string {
if orchestratorType == api.DCOS {
var buf bytes.Buffer
Expand Down
58 changes: 0 additions & 58 deletions pkg/acsengine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"io/ioutil"
"path"
"path/filepath"
"regexp"
"strings"
"testing"

Expand Down Expand Up @@ -506,41 +505,6 @@ func TestIsNSeriesSKU(t *testing.T) {
}
}

func TestIsCustomVNET(t *testing.T) {

a := []*api.AgentPoolProfile{
{
VnetSubnetID: "subnetlink1",
},
{
VnetSubnetID: "subnetlink2",
},
}

if !isCustomVNET(a) {
t.Fatalf("Expected isCustomVNET to be true when subnet exists for all agent pool profile")
}

a = []*api.AgentPoolProfile{
{
VnetSubnetID: "subnetlink1",
},
{
VnetSubnetID: "",
},
}

if isCustomVNET(a) {
t.Fatalf("Expected isCustomVNET to be false when subnet exists for some agent pool profile")
}

a = nil

if isCustomVNET(a) {
t.Fatalf("Expected isCustomVNET to be false when agent pool profiles is nil")
}
}

func TestGenerateIpList(t *testing.T) {
count := 3
forth := 240
Expand Down Expand Up @@ -592,25 +556,3 @@ func TestGenerateKubeConfig(t *testing.T) {
t.Fatalf("Expected an error result from nil Properties child properties")
}
}

func TestGenerateClusterID(t *testing.T) {
p := &api.Properties{
AgentPoolProfiles: []*api.AgentPoolProfile{
{
Name: "foo_agent",
},
},
}

clusterID := GenerateClusterID(p)

r, err := regexp.Compile("[0-9]{8}")

if err != nil {
t.Errorf("unexpected error while parsing regex : %s", err.Error())
}

if !r.MatchString(clusterID) {
t.Fatal("ClusterID should be an 8 digit integer string")
}
}
2 changes: 1 addition & 1 deletion pkg/acsengine/output.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ArtifactWriter struct {
// WriteTLSArtifacts saves TLS certificates and keys to the server filesystem
func (w *ArtifactWriter) WriteTLSArtifacts(containerService *api.ContainerService, apiVersion, template, parameters, artifactsDir string, certsGenerated bool, parametersOnly bool) error {
if len(artifactsDir) == 0 {
artifactsDir = fmt.Sprintf("%s-%s", containerService.Properties.OrchestratorProfile.OrchestratorType, GenerateClusterID(containerService.Properties))
artifactsDir = fmt.Sprintf("%s-%s", containerService.Properties.OrchestratorProfile.OrchestratorType, containerService.Properties.GenerateClusterID())
artifactsDir = path.Join("_output", artifactsDir)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/acsengine/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func TestWriteTLSArtifacts(t *testing.T) {
},
}
dir := "_testoutputdir"
defaultDir := fmt.Sprintf("%s-%s", cs.Properties.OrchestratorProfile.OrchestratorType, GenerateClusterID(cs.Properties))
defaultDir := fmt.Sprintf("%s-%s", cs.Properties.OrchestratorProfile.OrchestratorType, cs.Properties.GenerateClusterID())
defaultDir = path.Join("_output", defaultDir)
defer os.RemoveAll(dir)
defer os.RemoveAll(defaultDir)
Expand Down
8 changes: 1 addition & 7 deletions pkg/acsengine/params_k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,7 @@ func assignKubernetesParameters(properties *api.Properties, parametersMap params
}
}

if properties.HostedMasterProfile != nil {
addValue(parametersMap, "orchestratorName", "aks")
} else if properties.OrchestratorProfile.IsOpenShift() {
addValue(parametersMap, "orchestratorName", DefaultOpenshiftOrchestratorName)
} else {
addValue(parametersMap, "orchestratorName", DefaultOrchestratorName)
}
addValue(parametersMap, "orchestratorName", properties.K8sOrchestratorName())

/**
The following parameters could be either a plain text, or referenced to a secret in a keyvault:
Expand Down
18 changes: 15 additions & 3 deletions pkg/acsengine/template_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
return cs.Properties.MasterProfile != nil && cs.Properties.MasterProfile.IsVirtualMachineScaleSets()
},
"IsHostedMaster": func() bool {
return cs.Properties.HostedMasterProfile != nil
return cs.Properties.IsHostedMasterProfile()
},
"IsDCOS19": func() bool {
return cs.Properties.OrchestratorProfile.OrchestratorType == api.DCOS &&
Expand Down Expand Up @@ -395,7 +395,7 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
return getSecurityRules(ports)
},
"GetUniqueNameSuffix": func() string {
return GenerateClusterID(cs.Properties)
return cs.Properties.GenerateClusterID()
},
"GetVNETAddressPrefixes": func() string {
return getVNETAddressPrefixes(cs.Properties)
Expand Down Expand Up @@ -802,12 +802,24 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
imageRef := cs.Properties.MasterProfile.ImageRef
return imageRef != nil && len(imageRef.Name) > 0 && len(imageRef.ResourceGroup) > 0
},
"GetRouteTableName": func() string {
return cs.Properties.GetRouteTableName()
Copy link
Member

Choose a reason for hiding this comment

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

would it make more sense to have these methods directly on the ContainerService object to encapsulate/hide a bit more the Properties struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it should be tied to the Properties struct. What are thoughts on moving cs.Properties to a variable?

},
"GetNSGName": func() string {
return cs.Properties.GetNSGName()
},
"GetMasterEtcdServerPort": func() int {
return DefaultMasterEtcdServerPort
},
"GetMasterEtcdClientPort": func() int {
return DefaultMasterEtcdClientPort
},
"GetPrimaryAvailabilitySetName": func() string {
return cs.Properties.GetPrimaryAvailabilitySetName()
},
"GetPrimaryScaleSetName": func() string {
return cs.Properties.GetPrimaryScaleSetName()
},
"UseCloudControllerManager": func() bool {
return cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager != nil && *cs.Properties.OrchestratorProfile.KubernetesConfig.UseCloudControllerManager
},
Expand Down Expand Up @@ -907,7 +919,7 @@ func (t *TemplateGenerator) getTemplateFuncMap(cs *api.ContainerService) templat
return a - b
},
"IsCustomVNET": func() bool {
return isCustomVNET(cs.Properties.AgentPoolProfiles)
return cs.Properties.AreAgentProfilesCustomVNET()
},
"quote": strconv.Quote,
"shellQuote": func(s string) string {
Expand Down
18 changes: 17 additions & 1 deletion pkg/api/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ const (
const (
// AvailabilitySet means that the vms are in an availability set
AvailabilitySet = "AvailabilitySet"
// DefaultOrchestratorName specifies the 3 character orchestrator code of the cluster template and affects resource naming.
DefaultOrchestratorName = "k8s"
// DefaultOpenshiftOrchestratorName specifies the 3 character orchestrator code of the cluster template and affects resource naming.
DefaultOpenshiftOrchestratorName = "ocp"
// DefaultHostedProfileMasterName specifies the 3 character orchestrator code of the clusters with hosted master profiles.
DefaultHostedProfileMasterName = "aks"
// DefaultFirstConsecutiveKubernetesStaticIP specifies the static IP address on Kubernetes master 0
DefaultFirstConsecutiveKubernetesStaticIP = "10.240.255.5"
// DefaultFirstConsecutiveKubernetesStaticIPVMSS specifies the static IP address on Kubernetes master 0 of VMSS
Expand All @@ -72,6 +78,12 @@ const (
// DefaultKubernetesFirstConsecutiveStaticIPOffsetVMSS specifies the IP address offset of master 0 in VMSS
// when VNET integration is enabled.
DefaultKubernetesFirstConsecutiveStaticIPOffsetVMSS = 4
// DefaultSubnetNameResourceSegmentIndex specifies the default subnet name resource segment index.
DefaultSubnetNameResourceSegmentIndex = 10
// DefaultVnetResourceGroupSegmentIndex specifies the default virtual network resource segment index.
DefaultVnetResourceGroupSegmentIndex = 4
// DefaultVnetNameResourceSegmentIndex specifies the default virtual network name segment index.
DefaultVnetNameResourceSegmentIndex = 8
// VirtualMachineScaleSets means that the vms are in a virtual machine scaleset
VirtualMachineScaleSets = "VirtualMachineScaleSets"
// ScaleSetPriorityRegular is the default ScaleSet Priority
Expand Down Expand Up @@ -164,11 +176,15 @@ const (
NetworkPolicyNone = "none"
// NetworkPluginKubenet is the string expression for the kubenet NetworkPlugin config
NetworkPluginKubenet = "kubenet"
// NetworkPluginAzure is thee string expression for Azure CNI plugin.
// NetworkPluginAzure is the string expression for Azure CNI plugin.
NetworkPluginAzure = "azure"
// DefaultSinglePlacementGroup determines the acs-engine provided default for supporting large VMSS
// (true = single placement group 0-100 VMs, false = multiple placement group 0-1000 VMs)
DefaultSinglePlacementGroup = true
// ARMNetworkNamespace is the ARM-specific namespace for ARM's network providers.
ARMNetworkNamespace = "Microsoft.Networks"
// ARMVirtualNetworksResourceType is the ARM resource type for virtual network resources of ARM.
ARMVirtualNetworksResourceType = "virtualNetworks"
)

const (
Expand Down
Loading