Refactor all normalization code into new Normalize() method

This commit is contained in:
John Gardiner Myers 2022-10-30 14:09:06 -07:00
parent a105e74424
commit 6eed8ff095
25 changed files with 142 additions and 81 deletions

View File

@ -93,6 +93,7 @@ type AutoscalingGroup struct {
}
var _ fi.CompareWithID = &AutoscalingGroup{}
var _ fi.TaskNormalize = &AutoscalingGroup{}
// CompareWithID returns the ID of the ASG
func (e *AutoscalingGroup) CompareWithID() *string {
@ -311,20 +312,15 @@ func findAutoscalingGroup(cloud awsup.AWSCloud, name string) (*autoscaling.Group
return nil, fmt.Errorf("found multiple AutoscalingGroups with name: %q", name)
}
func (e *AutoscalingGroup) normalize(c *fi.Context) error {
func (e *AutoscalingGroup) Normalize(c *fi.Context) error {
sort.Strings(e.Metrics)
c.Cloud.(awsup.AWSCloud).AddTags(e.Name, e.Tags)
return nil
}
// Run is responsible for running the task
func (e *AutoscalingGroup) Run(c *fi.Context) error {
err := e.normalize(c)
if err != nil {
return err
}
c.Cloud.(awsup.AWSCloud).AddTags(e.Name, e.Tags)
return fi.DefaultDeltaRunMethod(e, c)
}

View File

@ -76,6 +76,7 @@ type ClassicLoadBalancer struct {
}
var _ fi.CompareWithID = &ClassicLoadBalancer{}
var _ fi.TaskNormalize = &ClassicLoadBalancer{}
func (e *ClassicLoadBalancer) CompareWithID() *string {
return e.Name
@ -332,8 +333,7 @@ func (e *ClassicLoadBalancer) Find(c *fi.Context) (*ClassicLoadBalancer, error)
e.LoadBalancerName = actual.LoadBalancerName
}
// TODO: Make Normalize a standard method
actual.Normalize()
_ = actual.Normalize(c)
klog.V(4).Infof("Found ELB %+v", actual)
@ -365,9 +365,6 @@ func (e *ClassicLoadBalancer) FindAddresses(context *fi.Context) ([]string, erro
}
func (e *ClassicLoadBalancer) Run(c *fi.Context) error {
// TODO: Make Normalize a standard method
e.Normalize()
return fi.DefaultDeltaRunMethod(e, c)
}
@ -378,10 +375,11 @@ func (_ *ClassicLoadBalancer) ShouldCreate(a, e, changes *ClassicLoadBalancer) (
return true, nil
}
func (e *ClassicLoadBalancer) Normalize() {
func (e *ClassicLoadBalancer) Normalize(c *fi.Context) error {
// We need to sort our arrays consistently, so we don't get spurious changes
sort.Stable(OrderSubnetsById(e.Subnets))
sort.Stable(OrderSecurityGroupsById(e.SecurityGroups))
return nil
}
func (s *ClassicLoadBalancer) CheckChanges(a, e, changes *ClassicLoadBalancer) error {

View File

@ -47,39 +47,15 @@ type EBSVolume struct {
}
var _ fi.CompareWithID = &EBSVolume{}
var _ fi.TaskNormalize = &EBSVolume{}
func (e *EBSVolume) CompareWithID() *string {
return e.ID
}
type TaggableResource interface {
FindResourceID(c fi.Cloud) (*string, error)
}
var _ TaggableResource = &EBSVolume{}
func (e *EBSVolume) FindResourceID(c fi.Cloud) (*string, error) {
actual, err := e.find(c.(awsup.AWSCloud))
if err != nil {
return nil, fmt.Errorf("error querying for EBSVolume: %v", err)
}
if actual == nil {
return nil, nil
}
return actual.ID, nil
}
func (e *EBSVolume) Find(context *fi.Context) (*EBSVolume, error) {
actual, err := e.find(context.Cloud.(awsup.AWSCloud))
if actual != nil && err == nil {
e.ID = actual.ID
}
cloud := context.Cloud.(awsup.AWSCloud)
return actual, err
}
func (e *EBSVolume) find(cloud awsup.AWSCloud) (*EBSVolume, error) {
filters := cloud.BuildFilters(e.Name)
request := &ec2.DescribeVolumesInput{
Filters: filters,
@ -116,11 +92,17 @@ func (e *EBSVolume) find(cloud awsup.AWSCloud) (*EBSVolume, error) {
// Avoid spurious changes
actual.Lifecycle = e.Lifecycle
e.ID = actual.ID
return actual, nil
}
func (e *EBSVolume) Run(c *fi.Context) error {
func (e *EBSVolume) Normalize(c *fi.Context) error {
c.Cloud.(awsup.AWSCloud).AddTags(e.Name, e.Tags)
return nil
}
func (e *EBSVolume) Run(c *fi.Context) error {
return fi.DefaultDeltaRunMethod(e, c)
}

View File

@ -93,6 +93,7 @@ type LaunchTemplate struct {
var (
_ fi.CompareWithID = &LaunchTemplate{}
_ fi.ProducesDeletions = &LaunchTemplate{}
_ fi.TaskNormalize = &LaunchTemplate{}
_ fi.Deletion = &deleteLaunchTemplate{}
)
@ -135,16 +136,14 @@ func (t *LaunchTemplate) buildRootDevice(cloud awsup.AWSCloud) (map[string]*Bloc
return bm, nil
}
// Run is responsible for
func (t *LaunchTemplate) Run(c *fi.Context) error {
t.Normalize()
return fi.DefaultDeltaRunMethod(t, c)
func (t *LaunchTemplate) Normalize(c *fi.Context) error {
sort.Stable(OrderSecurityGroupsById(t.SecurityGroups))
return nil
}
// Normalize is responsible for normalizing any data within the resource
func (t *LaunchTemplate) Normalize() {
sort.Stable(OrderSecurityGroupsById(t.SecurityGroups))
// Run is responsible for
func (t *LaunchTemplate) Run(c *fi.Context) error {
return fi.DefaultDeltaRunMethod(t, c)
}
// CheckChanges is responsible for ensuring certains fields

View File

@ -73,6 +73,7 @@ type NetworkLoadBalancer struct {
}
var _ fi.CompareWithID = &NetworkLoadBalancer{}
var _ fi.TaskNormalize = &NetworkLoadBalancer{}
func (e *NetworkLoadBalancer) CompareWithID() *string {
return e.Name
@ -418,8 +419,7 @@ func (e *NetworkLoadBalancer) Find(c *fi.Context) (*NetworkLoadBalancer, error)
e.LoadBalancerName = actual.LoadBalancerName
}
// TODO: Make Normalize a standard method
actual.Normalize()
_ = actual.Normalize(c)
actual.ForAPIServer = e.ForAPIServer
actual.Lifecycle = e.Lifecycle
@ -453,17 +453,15 @@ func (e *NetworkLoadBalancer) FindAddresses(context *fi.Context) ([]string, erro
}
func (e *NetworkLoadBalancer) Run(c *fi.Context) error {
// TODO: Make Normalize a standard method
e.Normalize()
return fi.DefaultDeltaRunMethod(e, c)
}
func (e *NetworkLoadBalancer) Normalize() {
func (e *NetworkLoadBalancer) Normalize(c *fi.Context) error {
// We need to sort our arrays consistently, so we don't get spurious changes
sort.Stable(OrderSubnetMappingsByName(e.SubnetMappings))
sort.Stable(OrderListenersByPort(e.Listeners))
sort.Stable(OrderTargetGroupsByName(e.TargetGroups))
return nil
}
func (*NetworkLoadBalancer) CheckChanges(a, e, changes *NetworkLoadBalancer) error {

View File

@ -47,6 +47,7 @@ type SSHKey struct {
}
var _ fi.CompareWithID = &SSHKey{}
var _ fi.TaskNormalize = &SSHKey{}
func (e *SSHKey) CompareWithID() *string {
return e.Name
@ -120,7 +121,7 @@ func (e *SSHKey) find(cloud awsup.AWSCloud) (*SSHKey, error) {
return actual, nil
}
func (e *SSHKey) Run(c *fi.Context) error {
func (e *SSHKey) Normalize(c *fi.Context) error {
if e.KeyFingerprint == nil && e.PublicKey != nil {
publicKey, err := fi.ResourceAsString(e.PublicKey)
if err != nil {
@ -135,6 +136,10 @@ func (e *SSHKey) Run(c *fi.Context) error {
e.KeyFingerprint = &keyFingerprint
}
return nil
}
func (e *SSHKey) Run(c *fi.Context) error {
return fi.DefaultDeltaRunMethod(e, c)
}

View File

@ -40,6 +40,7 @@ type Disk struct {
var (
_ fi.Task = &Disk{}
_ fi.CompareWithID = &Disk{}
_ fi.TaskNormalize = &Disk{}
)
// CompareWithID returns the Name of the Disk.
@ -76,9 +77,13 @@ func (d *Disk) Find(c *fi.Context) (*Disk, error) {
}, nil
}
func (d *Disk) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(d.Tags)
return nil
}
// Run implements fi.Task.Run.
func (d *Disk) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(d.Tags)
return fi.DefaultDeltaRunMethod(d, c)
}

View File

@ -138,7 +138,11 @@ func TestDiskRun(t *testing.T) {
}
vmss := newTestDisk()
err := vmss.Run(ctx)
err := vmss.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = vmss.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -45,6 +45,7 @@ type LoadBalancer struct {
var (
_ fi.Task = &LoadBalancer{}
_ fi.CompareWithID = &LoadBalancer{}
_ fi.TaskNormalize = &LoadBalancer{}
)
// CompareWithID returns the Name of the LoadBalancer
@ -98,9 +99,13 @@ func (lb *LoadBalancer) Find(c *fi.Context) (*LoadBalancer, error) {
}, nil
}
func (lb *LoadBalancer) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(lb.Tags)
return nil
}
// Run implements fi.Task.Run.
func (lb *LoadBalancer) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(lb.Tags)
return fi.DefaultDeltaRunMethod(lb, c)
}

View File

@ -145,7 +145,11 @@ func TestLoadBalancerRun(t *testing.T) {
}
lb := newTestLoadBalancer()
err := lb.Run(ctx)
err := lb.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = lb.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -39,6 +39,7 @@ type PublicIPAddress struct {
var (
_ fi.Task = &PublicIPAddress{}
_ fi.CompareWithID = &PublicIPAddress{}
_ fi.TaskNormalize = &PublicIPAddress{}
)
// CompareWithID returns the Name of the Public IP Address
@ -75,9 +76,13 @@ func (p *PublicIPAddress) Find(c *fi.Context) (*PublicIPAddress, error) {
}, nil
}
func (p *PublicIPAddress) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(p.Tags)
return nil
}
// Run implements fi.Task.Run.
func (p *PublicIPAddress) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(p.Tags)
return fi.DefaultDeltaRunMethod(p, c)
}

View File

@ -117,7 +117,11 @@ func TestPublicIPAddressRun(t *testing.T) {
}
lb := newTestPublicIPAddress()
err := lb.Run(ctx)
err := lb.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = lb.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -41,6 +41,7 @@ type ResourceGroup struct {
var (
_ fi.Task = &ResourceGroup{}
_ fi.CompareWithID = &ResourceGroup{}
_ fi.TaskNormalize = &ResourceGroup{}
)
// CompareWithID returns the Name of the VM Scale Set.
@ -74,9 +75,13 @@ func (r *ResourceGroup) Find(c *fi.Context) (*ResourceGroup, error) {
}, nil
}
func (r *ResourceGroup) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(r.Tags)
return nil
}
// Run implements fi.Task.Run.
func (r *ResourceGroup) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(r.Tags)
return fi.DefaultDeltaRunMethod(r, c)
}

View File

@ -156,7 +156,11 @@ func TestResourceGroupRun(t *testing.T) {
key: to.StringPtr(val),
},
}
err := rg.Run(ctx)
err := rg.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = rg.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -39,6 +39,7 @@ type RouteTable struct {
var (
_ fi.Task = &RouteTable{}
_ fi.CompareWithID = &RouteTable{}
_ fi.TaskNormalize = &RouteTable{}
)
// CompareWithID returns the Name of the VM Scale Set.
@ -73,9 +74,13 @@ func (r *RouteTable) Find(c *fi.Context) (*RouteTable, error) {
}, nil
}
func (r *RouteTable) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(r.Tags)
return nil
}
// Run implements fi.Task.Run.
func (r *RouteTable) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(r.Tags)
return fi.DefaultDeltaRunMethod(r, c)
}

View File

@ -43,6 +43,7 @@ type VirtualNetwork struct {
var (
_ fi.Task = &VirtualNetwork{}
_ fi.CompareWithID = &VirtualNetwork{}
_ fi.TaskNormalize = &VirtualNetwork{}
)
// CompareWithID returns the Name of the VM Scale Set.
@ -84,9 +85,13 @@ func (n *VirtualNetwork) Find(c *fi.Context) (*VirtualNetwork, error) {
}, nil
}
func (n *VirtualNetwork) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(n.Tags)
return nil
}
// Run implements fi.Task.Run.
func (n *VirtualNetwork) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(n.Tags)
return fi.DefaultDeltaRunMethod(n, c)
}

View File

@ -141,7 +141,11 @@ func TestVirtualNetworkRun(t *testing.T) {
key: to.StringPtr(val),
},
}
err := vnet.Run(ctx)
err := vnet.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = vnet.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -120,6 +120,8 @@ type VMScaleSet struct {
PrincipalID *string
}
var _ fi.TaskNormalize = &VMScaleSet{}
// VMScaleSetStorageProfile wraps *compute.VirtualMachineScaleSetStorageProfile
// and implements fi.HasDependencies.
//
@ -240,9 +242,13 @@ func (s *VMScaleSet) Find(c *fi.Context) (*VMScaleSet, error) {
return vmss, nil
}
func (s *VMScaleSet) Normalize(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(s.Tags)
return nil
}
// Run implements fi.Task.Run.
func (s *VMScaleSet) Run(c *fi.Context) error {
c.Cloud.(azure.AzureCloud).AddClusterTags(s.Tags)
return fi.DefaultDeltaRunMethod(s, c)
}

View File

@ -307,7 +307,11 @@ func TestVMScaleSetRun(t *testing.T) {
}
vmss := newTestVMScaleSet()
err := vmss.Run(ctx)
err := vmss.Normalize(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}
err = vmss.Run(ctx)
if err != nil {
t.Fatalf("unexpected error: %s", err)
}

View File

@ -48,6 +48,7 @@ type FirewallRule struct {
}
var _ fi.CompareWithID = &FirewallRule{}
var _ fi.TaskNormalize = &FirewallRule{}
func (e *FirewallRule) CompareWithID() *string {
return e.Name
@ -82,15 +83,12 @@ func (e *FirewallRule) Find(c *fi.Context) (*FirewallRule, error) {
}
func (e *FirewallRule) Run(c *fi.Context) error {
if err := e.sanityCheck(); err != nil {
return err
}
return fi.DefaultDeltaRunMethod(e, c)
}
// sanityCheck applies some validation that isn't technically required,
// Normalize applies some validation that isn't technically required,
// but avoids some problems with surprising behaviours.
func (e *FirewallRule) sanityCheck() error {
func (e *FirewallRule) Normalize(c *fi.Context) error {
if !e.Disabled {
// Treat it as an error if SourceRanges _and_ SourceTags empty with Disabled=false
// this is interpreted as SourceRanges="0.0.0.0/0", which is likely not what was intended.

View File

@ -39,6 +39,7 @@ type SSHKey struct {
}
var _ fi.CompareWithID = &SSHKey{}
var _ fi.TaskNormalize = &SSHKey{}
func (e *SSHKey) CompareWithID() *string {
return e.Name
@ -69,7 +70,7 @@ func (e *SSHKey) Find(c *fi.Context) (*SSHKey, error) {
return actual, nil
}
func (e *SSHKey) Run(c *fi.Context) error {
func (e *SSHKey) Normalize(c *fi.Context) error {
if e.KeyFingerprint == nil && e.PublicKey != nil {
publicKey, err := fi.ResourceAsString(e.PublicKey)
if err != nil {
@ -83,6 +84,10 @@ func (e *SSHKey) Run(c *fi.Context) error {
klog.V(2).Infof("Computed SSH key fingerprint as %q", keyFingerprint)
e.KeyFingerprint = &keyFingerprint
}
return nil
}
func (e *SSHKey) Run(c *fi.Context) error {
return fi.DefaultDeltaRunMethod(e, c)
}

View File

@ -37,6 +37,7 @@ type Volume struct {
}
var _ fi.CompareWithID = &Volume{}
var _ fi.TaskNormalize = &Volume{}
func (c *Volume) CompareWithID() *string {
return c.ID
@ -77,12 +78,15 @@ func (c *Volume) Find(context *fi.Context) (*Volume, error) {
return actual, nil
}
func (c *Volume) Run(context *fi.Context) error {
func (c *Volume) Normalize(context *fi.Context) error {
cloud := context.Cloud.(openstack.OpenstackCloud)
for k, v := range cloud.GetCloudTags() {
c.Tags[k] = v
}
return nil
}
func (c *Volume) Run(context *fi.Context) error {
return fi.DefaultDeltaRunMethod(c, context)
}

View File

@ -184,6 +184,14 @@ func (e *executor) forkJoin(tasks []*taskState) []error {
results[index] = fmt.Errorf("function panic")
defer wg.Done()
klog.V(2).Infof("Executing task %q: %v\n", ts.key, ts.task)
if taskNormalize, ok := ts.task.(TaskNormalize); ok {
if err := taskNormalize.Normalize(e.context); err != nil {
results[index] = err
return
}
}
results[index] = ts.task.Run(e.context)
}(tasks[i], i)
}

View File

@ -54,6 +54,7 @@ type Keypair struct {
var (
_ fi.HasCheckExisting = &Keypair{}
_ fi.HasName = &Keypair{}
_ fi.TaskNormalize = &Keypair{}
)
// It's important always to check for the existing key, so we don't regenerate keys e.g. on terraform
@ -115,14 +116,10 @@ func (e *Keypair) Find(c *fi.Context) (*Keypair, error) {
}
func (e *Keypair) Run(c *fi.Context) error {
err := e.normalize()
if err != nil {
return err
}
return fi.DefaultDeltaRunMethod(e, c)
}
func (e *Keypair) normalize() error {
func (e *Keypair) Normalize(c *fi.Context) error {
var alternateNames []string
for _, s := range e.AlternateNames {

View File

@ -30,10 +30,20 @@ type Task interface {
// TaskPreRun is implemented by tasks that perform some initial validation.
type TaskPreRun interface {
// PreRun will be run for all TaskPreRuns, before any Run functions are invoked.
Task
// PreRun will be run for all TaskPreRuns, before the Run function of any Task is invoked.
// Invocation order does not pay attention to Task dependencies.
PreRun(*Context) error
}
// TaskNormalize is implemented by tasks that perform some initial normalization.
type TaskNormalize interface {
Task
// Normalize will be run for all TaskNormalizes, before the Run function of
// the TaskNormalize and after the Run function of any Task it is dependent on.
Normalize(*Context) error
}
// TaskAsString renders the task for debug output
// TODO: Use reflection to make this cleaner: don't recurse into tasks - print their names instead
// also print resources in a cleaner way (use the resource source information?)
@ -42,6 +52,7 @@ func TaskAsString(t Task) string {
}
type HasCheckExisting interface {
Task
CheckExisting(c *Context) bool
}