Skip to content

Commit

Permalink
Merge pull request #3767 from patrickdillon/azure-session
Browse files Browse the repository at this point in the history
pkg/asset/installconfig/azure: create reusable session
  • Loading branch information
openshift-merge-robot committed Jun 19, 2020
2 parents 9eb7698 + faea85f commit 0fc9401
Show file tree
Hide file tree
Showing 16 changed files with 135 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
73 changes: 73 additions & 0 deletions pkg/asset/installconfig/azure/metadata.go
@@ -0,0 +1,73 @@
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 indicates the Azure cloud environment (e.g. public, gov't).
CloudName typesazure.CloudEnvironment `json:"cloudName,omitempty"`

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,12 +10,11 @@ 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"
"k8s.io/apimachinery/pkg/util/validation/field"

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

// Validate executes platform-specific validation.
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 0fc9401

Please sign in to comment.