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

auto-jumpbox provisioning for private clusters #2401

Merged
merged 17 commits into from
Mar 8, 2018

Conversation

CecileRobertMichon
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon commented Mar 6, 2018

What this PR does / why we need it: adds option to provision a jumpbox when enabling the private cluster. Some redesign of the private cluster config object.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #221

Special notes for your reviewer:

Release note:


This PR changes the cluster definition for private clusters to:

      "kubernetesConfig": {
        "privateCluster": {
          "enabled": true,
          "jumpboxProfile": {
            "name": "my-jb",
            "vmSize": "Standard_D4s_v3",
            "diskSizeGB": 30,
            "storageProfile": "ManagedDisks",
            "username": "azureuser",
            "publicKey": "xxx"
          }
      }

@CecileRobertMichon CecileRobertMichon changed the title [WIP] auto-jumpbox provisioning for private clusters auto-jumpbox provisioning for private clusters Mar 8, 2018
@CecileRobertMichon
Copy link
Contributor Author

@dennis-benzinger-hybris @yfried @SilverGaucho @MassimoSporchia @SilvioPilato FYI

@@ -294,7 +294,7 @@ func GenerateKubeConfig(properties *api.Properties, location string) (string, er
kubeconfig := string(b)
// variable replacement
kubeconfig = strings.Replace(kubeconfig, "{{WrapAsVerbatim \"variables('caCertificate')\"}}", base64.StdEncoding.EncodeToString([]byte(properties.CertificateProfile.CaCertificate)), -1)
if properties.OrchestratorProfile.KubernetesConfig.EnablePrivateCluster {
if *properties.OrchestratorProfile.KubernetesConfig.PrivateCluster.Enabled {
Copy link
Member

@jackfrancis jackfrancis Mar 8, 2018

Choose a reason for hiding this comment

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

We don't want the * reference here, if we want to have a simple boolean check (and if we know that properties.OrchestratorProfile.KubernetesConfig.PrivateCluster is non-nil), we can evaluate helpers.IsTrueBoolPointer(properties.OrchestratorProfile.KubernetesConfig.PrivateCluster.Enabled) as the if condition.

return cs.Properties.OrchestratorProfile.KubernetesConfig.UseManagedIdentity
},
"UseInstanceMetadata": func() bool {
return helpers.IsTrueBoolPointer(cs.Properties.OrchestratorProfile.KubernetesConfig.UseInstanceMetadata)
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 is there any reason why this wasn't being done before that I'm not aware of? I also set a default

Copy link
Member

Choose a reason for hiding this comment

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

This is a good change. 👍

@@ -93,6 +93,8 @@ const (
DefaultReschedulerAddonEnabled = false
// DefaultRBACEnabled determines the acs-engine provided default for enabling kubernetes RBAC
DefaultRBACEnabled = true
// DefaultUseInstanceMetadata determines the acs-engine provided default for enabling Azure cloudprovider instance metadata service
DefaultUseInstanceMetadata = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

default value for UseInstanceMetadata is true

@@ -443,6 +463,10 @@ func setOrchestratorDefaults(cs *api.ContainerService) {
a.OrchestratorProfile.KubernetesConfig.EnableSecureKubelet = pointerToBool(api.DefaultSecureKubeletEnabled)
}

if a.OrchestratorProfile.KubernetesConfig.UseInstanceMetadata == nil {
a.OrchestratorProfile.KubernetesConfig.EnableSecureKubelet = pointerToBool(api.DefaultUseInstanceMetadata)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

setting default for UseInstanceMetadata here

Copy link
Member

@jackfrancis jackfrancis left a comment

Choose a reason for hiding this comment

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

lgtm

return cs.Properties.OrchestratorProfile.KubernetesConfig.UseManagedIdentity
},
"UseInstanceMetadata": func() bool {
return helpers.IsTrueBoolPointer(cs.Properties.OrchestratorProfile.KubernetesConfig.UseInstanceMetadata)
Copy link
Member

Choose a reason for hiding this comment

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

This is a good change. 👍

@jackfrancis jackfrancis merged commit b0f5e38 into Azure:master Mar 8, 2018
@ghost ghost removed the in progress label Mar 8, 2018
@CecileRobertMichon CecileRobertMichon mentioned this pull request Mar 13, 2018
3 tasks
@CecileRobertMichon CecileRobertMichon deleted the auto-jumpbox branch March 15, 2018 19:02
tesharp pushed a commit to tesharp/acs-engine that referenced this pull request Mar 16, 2018
* add provision jumpbox flag

* add jp to resources

* fix osDisk resource

* Add jumpbox parameters

* fix broken link in docs

* fix nil reference error for agent pool only

* rename inconsistent params

* handle nil jumpbox profile

* Add support for storage account and managed disks

* docs

* update example files

* missing }

* fix typo in docs

* use isTrueBoolPointer

* keep instance metadata default

* fix typo

* check for nil private cluster
khaldoune added a commit to khaldoune/acs-engine that referenced this pull request Mar 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clusters without public IPs
2 participants