Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Determine default API access method by IG subnet type #14996

Merged
merged 2 commits into from
Jul 19, 2023
Merged

Determine default API access method by IG subnet type #14996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2420991
Determine default API access method by IG subnet type
johngmyers Jan 13, 2023
1c0b75a
hack/update-expected.sh
johngmyers Jan 13, 2023
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
2 changes: 1 addition & 1 deletion cmd/kops/create_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
}

assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), cluster.Spec.Assets, cluster.Spec.KubernetesVersion, false)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, cloud, assetBuilder)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, instanceGroups, cloud, assetBuilder)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ func updateCluster(ctx context.Context, clientset simple.Clientset, oldCluster,
}

assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), newCluster.Spec.Assets, newCluster.Spec.KubernetesVersion, false)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, newCluster, cloud, assetBuilder)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, newCluster, instanceGroups, cloud, assetBuilder)
if err != nil {
return fmt.Sprintf("error populating cluster spec: %s", err), nil
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/kops/edit_instancegroup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func updateInstanceGroup(ctx context.Context, clientset simple.Clientset, channe
}

assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), cluster.Spec.Assets, cluster.Spec.KubernetesVersion, false)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, cloud, assetBuilder)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, []*api.InstanceGroup{newGroup}, cloud, assetBuilder)
if err != nil {
return fmt.Sprintf("error populating cluster spec: %s", err), nil
}
Expand Down
6 changes: 3 additions & 3 deletions nodeup/pkg/model/kubelet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ func BuildNodeupModelContext(model *testutils.Model) (*NodeupModelContext, error
return nil, fmt.Errorf("error from PerformAssignments: %v", err)
}

nodeupModelContext.Cluster, err = mockedPopulateClusterSpec(ctx, model.Cluster, cloud)
nodeupModelContext.Cluster, err = mockedPopulateClusterSpec(ctx, model.Cluster, model.InstanceGroups, cloud)
if err != nil {
return nil, fmt.Errorf("unexpected error from mockedPopulateClusterSpec: %v", err)
}
Expand Down Expand Up @@ -289,7 +289,7 @@ func BuildNodeupModelContext(model *testutils.Model) (*NodeupModelContext, error
return nodeupModelContext, nil
}

func mockedPopulateClusterSpec(ctx context.Context, c *kops.Cluster, cloud fi.Cloud) (*kops.Cluster, error) {
func mockedPopulateClusterSpec(ctx context.Context, c *kops.Cluster, instanceGroups []*kops.InstanceGroup, cloud fi.Cloud) (*kops.Cluster, error) {
vfs.Context.ResetMemfsContext(true)

assetBuilder := assets.NewAssetBuilder(vfs.Context, c.Spec.Assets, c.Spec.KubernetesVersion, false)
Expand All @@ -298,7 +298,7 @@ func mockedPopulateClusterSpec(ctx context.Context, c *kops.Cluster, cloud fi.Cl
return nil, fmt.Errorf("error building vfspath: %v", err)
}
clientset := vfsclientset.NewVFSClientset(vfs.Context, basePath)
return cloudup.PopulateClusterSpec(ctx, clientset, c, cloud, assetBuilder)
return cloudup.PopulateClusterSpec(ctx, clientset, c, instanceGroups, cloud, assetBuilder)
}

// Fixed cert and key, borrowed from the create_kubecfg_test.go test
Expand Down
22 changes: 0 additions & 22 deletions pkg/apis/kops/v1alpha2/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1alpha2

import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"
)

func addDefaultingFuncs(scheme *runtime.Scheme) error {
Expand Down Expand Up @@ -54,27 +53,6 @@ func SetDefaults_ClusterSpec(obj *ClusterSpec) {
if obj.LegacyAPI == nil {
obj.LegacyAPI = &APISpec{}
}

if obj.LegacyAPI.IsEmpty() {
switch obj.Topology.ControlPlane {
case TopologyPublic:
obj.LegacyAPI.DNS = &DNSAccessSpec{}

case TopologyPrivate:
obj.LegacyAPI.LoadBalancer = &LoadBalancerAccessSpec{}

default:
klog.Infof("unknown master topology type: %q", obj.Topology.ControlPlane)
}
}

if obj.LegacyAPI.LoadBalancer != nil && obj.LegacyAPI.LoadBalancer.Type == "" {
obj.LegacyAPI.LoadBalancer.Type = LoadBalancerTypePublic
}

if obj.LegacyAPI.LoadBalancer != nil && obj.LegacyAPI.LoadBalancer.Class == "" && obj.LegacyCloudProvider == "aws" {
obj.LegacyAPI.LoadBalancer.Class = LoadBalancerClassClassic
}
}

if obj.Authorization == nil {
Expand Down
25 changes: 0 additions & 25 deletions pkg/apis/kops/v1alpha3/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1alpha3

import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"
)

func addDefaultingFuncs(scheme *runtime.Scheme) error {
Expand Down Expand Up @@ -46,30 +45,6 @@ func SetDefaults_ClusterSpec(obj *ClusterSpec) {
obj.Networking.Topology.DNS = DNSTypePublic
}

if obj.CloudProvider.Openstack == nil {
if obj.API.DNS == nil && obj.API.LoadBalancer == nil {
switch obj.Networking.Topology.ControlPlane {
case TopologyPublic:
obj.API.DNS = &DNSAccessSpec{}

case TopologyPrivate:
obj.API.LoadBalancer = &LoadBalancerAccessSpec{}

default:
klog.Infof("unknown controlPlane topology type: %q", obj.Networking.Topology.ControlPlane)
}
}

if obj.API.LoadBalancer != nil && obj.API.LoadBalancer.Type == "" {
obj.API.LoadBalancer.Type = LoadBalancerTypePublic
}

}

if obj.API.LoadBalancer != nil && obj.API.LoadBalancer.Class == "" && obj.CloudProvider.AWS != nil {
obj.API.LoadBalancer.Class = LoadBalancerClassClassic
}

if obj.Authorization == nil {
obj.Authorization = &AuthorizationSpec{}
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/apis/kops/validation/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ import (
"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
)

func awsValidateCluster(c *kops.Cluster) field.ErrorList {
func awsValidateCluster(c *kops.Cluster, strict bool) field.ErrorList {
allErrs := field.ErrorList{}

if c.Spec.API.LoadBalancer != nil {
lbPath := field.NewPath("spec", "api", "loadBalancer")
lbSpec := c.Spec.API.LoadBalancer
allErrs = append(allErrs, IsValidValue(lbPath.Child("class"), &lbSpec.Class, kops.SupportedLoadBalancerClasses)...)
if strict || lbSpec.Class != "" {
allErrs = append(allErrs, IsValidValue(lbPath.Child("class"), &lbSpec.Class, kops.SupportedLoadBalancerClasses)...)
}
allErrs = append(allErrs, awsValidateTopologyDNS(lbPath.Child("type"), c)...)
allErrs = append(allErrs, awsValidateSecurityGroupOverride(lbPath.Child("securityGroupOverride"), lbSpec)...)
allErrs = append(allErrs, awsValidateAdditionalSecurityGroups(lbPath.Child("additionalSecurityGroups"), lbSpec.AdditionalSecurityGroups)...)
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/kops/validation/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ func TestLoadBalancerSubnets(t *testing.T) {
})
}
cluster.Spec.API.LoadBalancer.Subnets = test.lbSubnets
errs := awsValidateCluster(&cluster)
errs := awsValidateCluster(&cluster, true)
testErrors(t, test, errs, test.expected)
}
}
Expand Down Expand Up @@ -670,7 +670,7 @@ func TestAWSAuthentication(t *testing.T) {
},
},
}
errs := awsValidateCluster(&cluster)
errs := awsValidateCluster(&cluster, true)
testErrors(t, test, errs, test.expected)
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/kops/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func newValidateCluster(cluster *kops.Cluster, strict bool) field.ErrorList {
// Additional cloud-specific validation rules
switch cluster.Spec.GetCloudProvider() {
case kops.CloudProviderAWS:
allErrs = append(allErrs, awsValidateCluster(cluster)...)
allErrs = append(allErrs, awsValidateCluster(cluster, strict)...)
case kops.CloudProviderGCE:
allErrs = append(allErrs, gceValidateCluster(cluster)...)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/helpers_readwrite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func UpdateCluster(ctx context.Context, clientset simple.Clientset, cluster *kop
}

assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), cluster.Spec.Assets, cluster.Spec.KubernetesVersion, false)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, cloud, assetBuilder)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, instanceGroups, cloud, assetBuilder)
if err != nil {
return err
}
Expand Down Expand Up @@ -79,7 +79,7 @@ func UpdateInstanceGroup(ctx context.Context, clientset simple.Clientset, cluste
}

assetBuilder := assets.NewAssetBuilder(clientset.VFSContext(), cluster.Spec.Assets, cluster.Spec.KubernetesVersion, false)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, cloud, assetBuilder)
fullCluster, err := cloudup.PopulateClusterSpec(ctx, clientset, cluster, allInstanceGroups, cloud, assetBuilder)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/instancegroups/rollingupdate_os_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func getTestSetupOS(t *testing.T, ctx context.Context) (*RollingUpdateCluster, *
assetBuilder := assets.NewAssetBuilder(vfs.Context, inCluster.Spec.Assets, inCluster.Spec.KubernetesVersion, false)
basePath, _ := vfs.Context.BuildVfsPath(inCluster.Spec.ConfigBase)
clientset := vfsclientset.NewVFSClientset(vfs.Context, basePath)
cluster, err := cloudup.PopulateClusterSpec(ctx, clientset, inCluster, mockcloud, assetBuilder)
cluster, err := cloudup.PopulateClusterSpec(ctx, clientset, inCluster, nil, mockcloud, assetBuilder)
if err != nil {
t.Fatalf("Failed to populate cluster spec: %v", err)
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/model/components/kubescheduler/tests/kubeschedulerconfig/completed-cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ metadata:
creationTimestamp: null
name: minimal.example.com
spec:
api:
dns: {}
api: {}
authorization:
alwaysAllow: {}
cloudProvider: {}
Expand Down
3 changes: 1 addition & 2 deletions pkg/model/components/kubescheduler/tests/minimal/completed-cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ metadata:
creationTimestamp: null
name: minimal.example.com
spec:
api:
dns: {}
api: {}
authorization:
alwaysAllow: {}
cloudProvider: {}
Expand Down
3 changes: 1 addition & 2 deletions pkg/model/components/kubescheduler/tests/mixing/completed-cluster.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ metadata:
creationTimestamp: null
name: minimal.example.com
spec:
api:
dns: {}
api: {}
authorization:
alwaysAllow: {}
cloudProvider: {}
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/apply_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error {

// upgradeSpecs ensures that fields are fully populated / defaulted
func (c *ApplyClusterCmd) upgradeSpecs(ctx context.Context, assetBuilder *assets.AssetBuilder) error {
fullCluster, err := PopulateClusterSpec(ctx, c.Clientset, c.Cluster, c.Cloud, assetBuilder)
fullCluster, err := PopulateClusterSpec(ctx, c.Clientset, c.Cluster, c.InstanceGroups, c.Cloud, assetBuilder)
if err != nil {
return err
}
Expand Down
53 changes: 49 additions & 4 deletions upup/pkg/fi/cloudup/populate_cluster_spec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ type populateClusterSpec struct {
// We build it up into a complete config, but we write the values as input
InputCluster *kopsapi.Cluster

// InputInstanceGroups are the instance groups in the cluster
InputInstanceGroups []*kopsapi.InstanceGroup

// fullCluster holds the built completed cluster spec
fullCluster *kopsapi.Cluster

Expand All @@ -53,11 +56,12 @@ type populateClusterSpec struct {

// PopulateClusterSpec takes a user-specified cluster spec, and computes the full specification that should be set on the cluster.
// We do this so that we don't need any real "brains" on the node side.
func PopulateClusterSpec(ctx context.Context, clientset simple.Clientset, cluster *kopsapi.Cluster, cloud fi.Cloud, assetBuilder *assets.AssetBuilder) (*kopsapi.Cluster, error) {
func PopulateClusterSpec(ctx context.Context, clientset simple.Clientset, cluster *kopsapi.Cluster, instanceGroups []*kopsapi.InstanceGroup, cloud fi.Cloud, assetBuilder *assets.AssetBuilder) (*kopsapi.Cluster, error) {
c := &populateClusterSpec{
cloud: cloud,
InputCluster: cluster,
assetBuilder: assetBuilder,
cloud: cloud,
InputCluster: cluster,
InputInstanceGroups: instanceGroups,
assetBuilder: assetBuilder,
}
err := c.run(ctx, clientset)
if err != nil {
Expand Down Expand Up @@ -217,6 +221,47 @@ func (c *populateClusterSpec) run(ctx context.Context, clientset simple.Clientse
klog.V(2).Infof("Normalizing kubernetes version: %q -> %q", cluster.Spec.KubernetesVersion, versionWithoutV)
cluster.Spec.KubernetesVersion = versionWithoutV
}

if cluster.Spec.CloudProvider.Openstack == nil {
if cluster.Spec.API.DNS == nil && cluster.Spec.API.LoadBalancer == nil {
subnetTypesByName := map[string]kopsapi.SubnetType{}
for _, subnet := range cluster.Spec.Networking.Subnets {
subnetTypesByName[subnet.Name] = subnet.Type
}

haveAPIServerNodes := false
for _, ig := range c.InputInstanceGroups {
if ig.Spec.Role == kopsapi.InstanceGroupRoleAPIServer {
haveAPIServerNodes = true
}
}
for _, ig := range c.InputInstanceGroups {
if ig.Spec.Role == kopsapi.InstanceGroupRoleAPIServer || (!haveAPIServerNodes && ig.Spec.Role == kopsapi.InstanceGroupRoleControlPlane) {
for _, subnet := range ig.Spec.Subnets {
switch subnetTypesByName[subnet] {
case kopsapi.SubnetTypePrivate:
cluster.Spec.API.LoadBalancer = &kopsapi.LoadBalancerAccessSpec{}
case kopsapi.SubnetTypePublic:
cluster.Spec.API.DNS = &kopsapi.DNSAccessSpec{}
}
}
}
}
// If both public and private, go with the load balancer.
if cluster.Spec.API.LoadBalancer != nil {
cluster.Spec.API.DNS = nil
}
}

if cluster.Spec.API.LoadBalancer != nil && cluster.Spec.API.LoadBalancer.Type == "" {
cluster.Spec.API.LoadBalancer.Type = kopsapi.LoadBalancerTypePublic
}
}

if cluster.Spec.API.LoadBalancer != nil && cluster.Spec.API.LoadBalancer.Class == "" && cluster.Spec.CloudProvider.AWS != nil {
cluster.Spec.API.LoadBalancer.Class = kopsapi.LoadBalancerClassClassic
}

if cluster.Spec.DNSZone == "" && cluster.PublishesDNSRecords() {
dns, err := cloud.DNS()
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/populate_cluster_spec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func mockedPopulateClusterSpec(ctx context.Context, c *kopsapi.Cluster, cloud fi
return nil, fmt.Errorf("error building vfspath: %v", err)
}
clientset := vfsclientset.NewVFSClientset(vfs.Context, basePath)
return PopulateClusterSpec(ctx, clientset, c, cloud, assetBuilder)
return PopulateClusterSpec(ctx, clientset, c, nil, cloud, assetBuilder)
}

func TestPopulateCluster_Docker_Spec(t *testing.T) {
Expand Down
Loading