Skip to content

Commit

Permalink
pkg/asset/installconfig/azure: create reusable session
Browse files Browse the repository at this point in the history
This commit creates an Azure metadata struct in the installconfig asset
(similar to AWS) and stores an Azure session and clients for interacting
with the Azure API. This commit also refactors existing code to use
this functionality rather than plumbing the install config values repeatedly.
  • Loading branch information
patrickdillon committed Jun 17, 2020
1 parent 65b9ee9 commit cd3507b
Show file tree
Hide file tree
Showing 16 changed files with 133 additions and 80 deletions.
12 changes: 6 additions & 6 deletions pkg/asset/cluster/tfvars.go
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/openshift/installer/pkg/asset/ignition/machine"
"github.com/openshift/installer/pkg/asset/installconfig"
awsconfig "github.com/openshift/installer/pkg/asset/installconfig/aws"
azureconfig "github.com/openshift/installer/pkg/asset/installconfig/azure"
gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
openstackconfig "github.com/openshift/installer/pkg/asset/installconfig/openstack"
ovirtconfig "github.com/openshift/installer/pkg/asset/installconfig/ovirt"
Expand Down Expand Up @@ -247,15 +246,16 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
Data: data,
})
case azure.Name:
sess, err := azureconfig.GetSession(installConfig.Config.Platform.Azure.CloudName)
session, err := installConfig.Azure.Session()
if err != nil {
return err
}

auth := azuretfvars.Auth{
SubscriptionID: sess.Credentials.SubscriptionID,
ClientID: sess.Credentials.ClientID,
ClientSecret: sess.Credentials.ClientSecret,
TenantID: sess.Credentials.TenantID,
SubscriptionID: session.Credentials.SubscriptionID,
ClientID: session.Credentials.ClientID,
ClientSecret: session.Credentials.ClientSecret,
TenantID: session.Credentials.TenantID,
}
masters, err := mastersAsset.Machines()
if err != nil {
Expand Down
6 changes: 4 additions & 2 deletions pkg/asset/installconfig/azure/azure.go
Expand Up @@ -20,13 +20,15 @@ const (

// Platform collects azure-specific configuration.
func Platform() (*azure.Platform, error) {
// Create client using public cloud because install config has not been generated yet.
const cloudName = azure.PublicCloud

client, err := NewClient(context.TODO(), cloudName)
ssn, err := GetSession(cloudName)
if err != nil {
return nil, err
}

client := NewClient(ssn)

regions, err := getRegions(context.TODO(), client)
if err != nil {
return nil, errors.Wrap(err, "failed to get list of regions")
Expand Down
17 changes: 3 additions & 14 deletions pkg/asset/installconfig/azure/client.go
Expand Up @@ -5,15 +5,12 @@ import (
"strings"
"time"

"github.com/pkg/errors"

azsku "github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2018-10-01/compute"
aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
azres "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-05-01/resources"
azsubs "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-06-01/subscriptions"
"github.com/Azure/go-autorest/autorest/to"

"github.com/openshift/installer/pkg/types/azure"
"github.com/pkg/errors"
)

//go:generate mockgen -source=./client.go -destination=mock/azureclient_generated.go -package=mock
Expand All @@ -34,19 +31,11 @@ type Client struct {
}

// NewClient initializes a client with a session.
func NewClient(ctx context.Context, cloudName azure.CloudEnvironment) (*Client, error) {
ctx, cancel := context.WithTimeout(ctx, 1*time.Minute)
defer cancel()

ssn, err := GetSession(cloudName)
if err != nil {
return nil, errors.Wrap(err, "failed to get session")
}

func NewClient(ssn *Session) *Client {
client := &Client{
ssn: ssn,
}
return client, nil
return client
}

// GetVirtualNetwork gets an Azure virtual network by name
Expand Down
20 changes: 7 additions & 13 deletions pkg/asset/installconfig/azure/dns.go
Expand Up @@ -9,13 +9,11 @@ import (
"github.com/Azure/go-autorest/autorest/to"
"github.com/pkg/errors"
survey "gopkg.in/AlecAivazis/survey.v1"

"github.com/openshift/installer/pkg/types/azure"
)

//DNSConfig exposes functions to choose the DNS settings
type DNSConfig struct {
Session *Session
session *Session
}

//ZonesGetter fetches the DNS zones available for the installer
Expand Down Expand Up @@ -64,7 +62,7 @@ func transformZone(f func(s string) *Zone) survey.Transformer {
func (config DNSConfig) GetDNSZoneID(rgName string, zoneName string) string {
return fmt.Sprintf(
"/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/dnszones/%s",
config.Session.Credentials.SubscriptionID,
config.session.Credentials.SubscriptionID,
rgName,
zoneName)
}
Expand All @@ -74,15 +72,15 @@ func (config DNSConfig) GetDNSZoneID(rgName string, zoneName string) string {
func (config DNSConfig) GetPrivateDNSZoneID(rgName string, zoneName string) string {
return fmt.Sprintf(
"/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/privateDnsZones/%s",
config.Session.Credentials.SubscriptionID,
config.session.Credentials.SubscriptionID,
rgName,
zoneName)
}

//GetDNSZone returns a DNS zone selected by survey
func (config DNSConfig) GetDNSZone() (*Zone, error) {
//call azure api using the session to retrieve available base domain
zonesClient := newZonesClient(config.Session)
zonesClient := newZonesClient(config.session)
allZones, _ := zonesClient.GetAllPublicZones()
if len(allZones) == 0 {
return nil, errors.New("no public dns zone found in your subscription")
Expand Down Expand Up @@ -115,19 +113,15 @@ func (config DNSConfig) GetDNSZone() (*Zone, error) {

//GetDNSRecordSet gets a record set for the zone identified by publicZoneID
func (config DNSConfig) GetDNSRecordSet(rgName string, zoneName string, relativeRecordSetName string, recordType azdns.RecordType) (*azdns.RecordSet, error) {
recordsetsClient := newRecordSetsClient(config.Session)
recordsetsClient := newRecordSetsClient(config.session)
return recordsetsClient.GetRecordSet(rgName, zoneName, relativeRecordSetName, recordType)
}

//NewDNSConfig returns a new DNSConfig struct that helps configuring the DNS
//by querying your subscription and letting you choose
//which domain you wish to use for the cluster
func NewDNSConfig(cloudName azure.CloudEnvironment) (*DNSConfig, error) {
session, err := GetSession(cloudName)
if err != nil {
return nil, errors.Wrap(err, "could not retrieve session information")
}
return &DNSConfig{Session: session}, nil
func NewDNSConfig(ssn *Session) *DNSConfig {
return &DNSConfig{session: ssn}
}

func newZonesClient(session *Session) ZonesGetter {
Expand Down
71 changes: 71 additions & 0 deletions pkg/asset/installconfig/azure/metadata.go
@@ -0,0 +1,71 @@
package azure

import (
"sync"

"github.com/pkg/errors"

typesazure "github.com/openshift/installer/pkg/types/azure"
)

// Metadata holds additional metadata for InstallConfig resources that
// does not need to be user-supplied (e.g. because it can be retrieved
// from external APIs).
type Metadata struct {
session *Session
client *Client
dnsCfg *DNSConfig
cloudName typesazure.CloudEnvironment

mutex sync.Mutex
}

// NewMetadata initializes a new Metadata object.
func NewMetadata(cloudName typesazure.CloudEnvironment) *Metadata {
return &Metadata{cloudName: cloudName}
}

// Session holds an Azure session which can be used for Azure API calls
// during asset generation.
func (m *Metadata) Session() (*Session, error) {
m.mutex.Lock()
defer m.mutex.Unlock()

return m.unlockedSession()
}

func (m *Metadata) unlockedSession() (*Session, error) {
if m.session == nil {
var err error
m.session, err = GetSession(m.cloudName)
if err != nil {
return nil, errors.Wrap(err, "creating Azure session")
}
}

return m.session, nil
}

// Client holds an Azure Client that implements calls to the Azure API.
func (m *Metadata) Client() (*Client, error) {
if m.client == nil {
ssn, err := m.Session()
if err != nil {
return nil, err
}
m.client = NewClient(ssn)
}
return m.client, nil
}

// DNSConfig holds an Azure DNSConfig Client that implements calls to the Azure API.
func (m *Metadata) DNSConfig() (*DNSConfig, error) {
if m.dnsCfg == nil {
ssn, err := m.Session()
if err != nil {
return nil, err
}
m.dnsCfg = NewDNSConfig(ssn)
}
return m.dnsCfg, nil
}
19 changes: 4 additions & 15 deletions pkg/asset/installconfig/azure/validation.go
Expand Up @@ -10,11 +10,10 @@ import (
azdns "github.com/Azure/azure-sdk-for-go/profiles/latest/dns/mgmt/dns"
aznetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-12-01/network"
"github.com/Azure/go-autorest/autorest/to"
aztypes "github.com/openshift/installer/pkg/types/azure"
"github.com/pkg/errors"


aztypes "github.com/openshift/installer/pkg/types/azure"
"github.com/openshift/installer/pkg/types"
azuretypes "github.com/openshift/installer/pkg/types/azure"
"k8s.io/apimachinery/pkg/util/validation/field"
)

Expand Down Expand Up @@ -122,15 +121,11 @@ func validateRegion(client API, fieldPath *field.Path, p *aztypes.Platform) fiel
}

// validateRegionForUltraDisks checks that the Ultra SSD disks are available for the user.
func validateRegionForUltraDisks(fldPath *field.Path, cloudName azuretypes.CloudEnvironment, region string) *field.Error {
func validateRegionForUltraDisks(fldPath *field.Path, client *Client, region string) *field.Error {
diskType := "UltraSSD_LRS"

ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
defer cancel()
client, err := NewClient(ctx, cloudName)
if err != nil {
return field.InternalError(fldPath.Child("diskType"), err)
}

regionDisks, err := client.GetDiskSkus(ctx, region)
if err != nil {
Expand All @@ -150,20 +145,14 @@ func validateRegionForUltraDisks(fldPath *field.Path, cloudName azuretypes.Cloud

// ValidatePublicDNS checks DNS for CNAME, A, and AAA records for
// api.zoneName. If a record exists, it's likely a cluster already exists.
func ValidatePublicDNS(ic *types.InstallConfig) error {
func ValidatePublicDNS(ic *types.InstallConfig, azureDNS *DNSConfig) error {
// If this is an internal cluster, this check is not necessary
if ic.Publish == types.InternalPublishingStrategy {
return nil
}

clusterName := ic.ObjectMeta.Name
record := fmt.Sprintf("api.%s", clusterName)

azureDNS, err := NewDNSConfig(ic.Azure.CloudName)
if err != nil {
return err
}

rgName := ic.Azure.BaseDomainResourceGroupName
zoneName := ic.BaseDomain
fmtStr := "api.%s %s record already exists in %s and might be in use by another cluster, please remove it to continue"
Expand Down
11 changes: 7 additions & 4 deletions pkg/asset/installconfig/basedomain.go
Expand Up @@ -33,25 +33,28 @@ func (a *baseDomain) Generate(parents asset.Parents) error {
platform := &platform{}
parents.Get(platform)

var err error
switch platform.CurrentName() {
case aws.Name:
var err error
a.BaseDomain, err = awsconfig.GetBaseDomain()
cause := errors.Cause(err)
if !(awsconfig.IsForbidden(cause) || request.IsErrorThrottle(cause)) {
return err
}
case azure.Name:
var err error
azureDNS, _ := azureconfig.NewDNSConfig(azure.PublicCloud)
// Create client using public cloud because install config has not been generated yet.
ssn, err := azureconfig.GetSession(azure.PublicCloud)
if err != nil {
return err
}
azureDNS := azureconfig.NewDNSConfig(ssn)
zone, err := azureDNS.GetDNSZone()
if err != nil {
return err
}
a.BaseDomain = zone.Name
return platform.Azure.SetBaseDomain(zone.ID)
case gcp.Name:
var err error
a.BaseDomain, err = gcpconfig.GetBaseDomain(platform.GCP.ProjectID)

// We are done if success (err == nil) or an err besides forbidden/throttling
Expand Down
7 changes: 5 additions & 2 deletions pkg/asset/installconfig/installconfig.go
Expand Up @@ -31,6 +31,7 @@ type InstallConfig struct {
Config *types.InstallConfig `json:"config"`
File *asset.File `json:"file"`
AWS *aws.Metadata `json:"aws,omitempty"`
Azure *icazure.Metadata `json:"azure,omitempty`
}

var _ asset.WritableAsset = (*InstallConfig)(nil)
Expand Down Expand Up @@ -134,7 +135,9 @@ func (a *InstallConfig) finish(filename string) error {
if a.Config.AWS != nil {
a.AWS = aws.NewMetadata(a.Config.Platform.AWS.Region, a.Config.Platform.AWS.Subnets, a.Config.AWS.ServiceEndpoints)
}

if a.Config.Azure != nil {
a.Azure = icazure.NewMetadata(a.Config.Azure.CloudName)
}
if err := validation.ValidateInstallConfig(a.Config, icopenstack.NewValidValuesFetcher()).ToAggregate(); err != nil {
if filename == "" {
return errors.Wrap(err, "invalid install config")
Expand All @@ -159,7 +162,7 @@ func (a *InstallConfig) finish(filename string) error {

func (a *InstallConfig) platformValidation() error {
if a.Config.Platform.Azure != nil {
client, err := icazure.NewClient(context.TODO(), a.Config.Platform.Azure.CloudName)
client, err := a.Azure.Client()
if err != nil {
return err
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/asset/installconfig/platformcredscheck.go
Expand Up @@ -7,7 +7,6 @@ import (
"github.com/pkg/errors"

"github.com/openshift/installer/pkg/asset"
azureconfig "github.com/openshift/installer/pkg/asset/installconfig/azure"
gcpconfig "github.com/openshift/installer/pkg/asset/installconfig/gcp"
openstackconfig "github.com/openshift/installer/pkg/asset/installconfig/openstack"
ovirtconfig "github.com/openshift/installer/pkg/asset/installconfig/ovirt"
Expand Down Expand Up @@ -60,7 +59,7 @@ func (a *PlatformCredsCheck) Generate(dependencies asset.Parents) error {
case baremetal.Name, libvirt.Name, none.Name, vsphere.Name:
// no creds to check
case azure.Name:
_, err = azureconfig.GetSession(ic.Config.Azure.CloudName)
_, err = ic.Azure.Session()
if err != nil {
return errors.Wrap(err, "creating Azure session")
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/asset/installconfig/platformprovisioncheck.go
Expand Up @@ -47,7 +47,11 @@ func (a *PlatformProvisionCheck) Generate(dependencies asset.Parents) error {
return err
}
case azure.Name:
err = azconfig.ValidatePublicDNS(ic.Config)
dnsConfig, err := ic.Azure.DNSConfig()
if err != nil {
return err
}
err = azconfig.ValidatePublicDNS(ic.Config, dnsConfig)
if err != nil {
return err
}
Expand Down

0 comments on commit cd3507b

Please sign in to comment.