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

[Federation][kubefed] Add option to disable persistence storage for etcd #40862

Merged
merged 1 commit into from Feb 3, 2017
Merged

[Federation][kubefed] Add option to disable persistence storage for etcd #40862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 33 additions & 21 deletions federation/pkg/kubefed/init/init.go
View file
Open in desktop
Expand Up @@ -134,6 +134,7 @@ func NewCmdInit(cmdOut io.Writer, config util.AdminConfig) *cobra.Command {
cmd.Flags().String("image", defaultImage, "Image to use for federation API server and controller manager binaries.")
cmd.Flags().String("dns-provider", "google-clouddns", "Dns provider to be used for this deployment.")
cmd.Flags().String("etcd-pv-capacity", "10Gi", "Size of persistent volume claim to be used for etcd.")
cmd.Flags().Bool("etcd-persistent-storage", true, "Use persistent volume for etcd. Defaults to 'true'.")
cmd.Flags().Bool("dry-run", false, "dry run without sending commands to server.")
cmd.Flags().String("storage-backend", "etcd2", "The storage backend for persistence. Options: 'etcd2' (default), 'etcd3'.")
return cmd
Expand All @@ -158,6 +159,7 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman
image := cmdutil.GetFlagString(cmd, "image")
dnsProvider := cmdutil.GetFlagString(cmd, "dns-provider")
etcdPVCapacity := cmdutil.GetFlagString(cmd, "etcd-pv-capacity")
etcdPersistence := cmdutil.GetFlagBool(cmd, "etcd-persistent-storage")
dryRun := cmdutil.GetDryRunFlag(cmd)
storageBackend := cmdutil.GetFlagString(cmd, "storage-backend")

Expand Down Expand Up @@ -208,9 +210,12 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman
// 5. Create a persistent volume and a claim to store the federation
// API server's state. This is where federation API server's etcd
// stores its data.
pvc, err := createPVC(hostClientset, initFlags.FederationSystemNamespace, svc.Name, etcdPVCapacity, dryRun)
if err != nil {
return err
var pvc *api.PersistentVolumeClaim
if etcdPersistence {
pvc, err = createPVC(hostClientset, initFlags.FederationSystemNamespace, svc.Name, etcdPVCapacity, dryRun)
if err != nil {
return err
}
}

// Since only one IP address can be specified as advertise address,
Expand All @@ -226,7 +231,7 @@ func initFederation(cmdOut io.Writer, config util.AdminConfig, cmd *cobra.Comman
}

// 6. Create federation API server
_, err = createAPIServer(hostClientset, initFlags.FederationSystemNamespace, serverName, image, serverCredName, pvc.Name, advertiseAddress, storageBackend, dryRun)
_, err = createAPIServer(hostClientset, initFlags.FederationSystemNamespace, serverName, image, serverCredName, advertiseAddress, storageBackend, pvc, dryRun)
if err != nil {
return err
}
Expand Down Expand Up @@ -445,7 +450,7 @@ func createPVC(clientset *client.Clientset, namespace, svcName, etcdPVCapacity s
return clientset.Core().PersistentVolumeClaims(namespace).Create(pvc)
}

func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, pvcName, advertiseAddress, storageBackend string, dryRun bool) (*extensions.Deployment, error) {
func createAPIServer(clientset *client.Clientset, namespace, name, image, credentialsName, advertiseAddress, storageBackend string, pvc *api.PersistentVolumeClaim, dryRun bool) (*extensions.Deployment, error) {
command := []string{
"/hyperkube",
"federation-apiserver",
Expand All @@ -462,8 +467,6 @@ func createAPIServer(clientset *client.Clientset, namespace, name, image, creden
command = append(command, fmt.Sprintf("--advertise-address=%s", advertiseAddress))
}

dataVolumeName := "etcddata"

dep := &extensions.Deployment{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Expand Down Expand Up @@ -509,12 +512,6 @@ func createAPIServer(clientset *client.Clientset, namespace, name, image, creden
"--data-dir",
"/var/etcd/data",
},
VolumeMounts: []api.VolumeMount{
{
Name: dataVolumeName,
MountPath: "/var/etcd",
},
},
},
},
Volumes: []api.Volume{
Expand All @@ -526,20 +523,35 @@ func createAPIServer(clientset *client.Clientset, namespace, name, image, creden
},
},
},
{
Name: dataVolumeName,
VolumeSource: api.VolumeSource{
PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{
ClaimName: pvcName,
},
},
},
},
},
},
},
}

if pvc != nil {
dataVolumeName := "etcddata"
etcdVolume := api.Volume{
Name: dataVolumeName,
VolumeSource: api.VolumeSource{
PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{
ClaimName: pvc.Name,
},
},
}
etcdVolumeMount := api.VolumeMount{
Name: dataVolumeName,
MountPath: "/var/etcd",
}

dep.Spec.Template.Spec.Volumes = append(dep.Spec.Template.Spec.Volumes, etcdVolume)
for i, container := range dep.Spec.Template.Spec.Containers {
if container.Name == "etcd" {
dep.Spec.Template.Spec.Containers[i].VolumeMounts = append(dep.Spec.Template.Spec.Containers[i].VolumeMounts, etcdVolumeMount)
}
}
}

if dryRun {
return dep, nil
}
Expand Down
48 changes: 32 additions & 16 deletions federation/pkg/kubefed/init/init_test.go
View file
Open in desktop
Expand Up @@ -79,6 +79,7 @@ func TestInitFederation(t *testing.T) {
lbIP string
image string
etcdPVCapacity string
etcdPersistence string
expectedErr string
dnsProvider string
storageBackend string
Expand All @@ -92,6 +93,7 @@ func TestInitFederation(t *testing.T) {
lbIP: "10.20.30.40",
image: "example.test/foo:bar",
etcdPVCapacity: "5Gi",
etcdPersistence: "true",
expectedErr: "",
dnsProvider: "test-dns-provider",
storageBackend: "etcd2",
Expand All @@ -105,6 +107,7 @@ func TestInitFederation(t *testing.T) {
lbIP: "10.20.30.40",
image: "example.test/foo:bar",
etcdPVCapacity: "", //test for default value of pvc-size
etcdPersistence: "true",
expectedErr: "",
dnsProvider: "", //test for default value of dns provider
storageBackend: "etcd2",
Expand All @@ -118,6 +121,7 @@ func TestInitFederation(t *testing.T) {
lbIP: "10.20.30.40",
image: "example.test/foo:bar",
etcdPVCapacity: "",
etcdPersistence: "true",
expectedErr: "",
dnsProvider: "test-dns-provider",
storageBackend: "etcd2",
Expand All @@ -131,6 +135,7 @@ func TestInitFederation(t *testing.T) {
lbIP: "10.20.30.40",
image: "example.test/foo:bar",
etcdPVCapacity: "5Gi",
etcdPersistence: "false",
expectedErr: "",
dnsProvider: "test-dns-provider",
storageBackend: "etcd3",
Expand All @@ -150,7 +155,7 @@ func TestInitFederation(t *testing.T) {
} else {
dnsProvider = "google-clouddns" //default value of dns-provider
}
hostFactory, err := fakeInitHostFactory(tc.federation, util.DefaultFederationSystemNamespace, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.etcdPVCapacity, tc.storageBackend)
hostFactory, err := fakeInitHostFactory(tc.federation, util.DefaultFederationSystemNamespace, tc.lbIP, tc.dnsZoneName, tc.image, dnsProvider, tc.etcdPersistence, tc.etcdPVCapacity, tc.storageBackend)
if err != nil {
t.Fatalf("[%d] unexpected error: %v", i, err)
}
Expand All @@ -175,6 +180,9 @@ func TestInitFederation(t *testing.T) {
if tc.etcdPVCapacity != "" {
cmd.Flags().Set("etcd-pv-capacity", tc.etcdPVCapacity)
}
if tc.etcdPersistence != "true" {
cmd.Flags().Set("etcd-persistent-storage", tc.etcdPersistence)
}
if tc.dryRun == "valid-run" {
cmd.Flags().Set("dry-run", "true")
}
Expand Down Expand Up @@ -445,7 +453,7 @@ func TestCertsHTTPS(t *testing.T) {
}
}

func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, dnsProvider, etcdPVCapacity, storageProvider string) (cmdutil.Factory, error) {
func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image, dnsProvider, etcdPersistence, etcdPVCapacity, storageProvider string) (cmdutil.Factory, error) {
svcName := federationName + "-apiserver"
svcUrlPrefix := "/api/v1/namespaces/federation-system/services"
credSecretName := svcName + "-credentials"
Expand Down Expand Up @@ -673,12 +681,6 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image,
"--data-dir",
"/var/etcd/data",
},
VolumeMounts: []v1.VolumeMount{
{
Name: "etcddata",
MountPath: "/var/etcd",
},
},
},
},
Volumes: []v1.Volume{
Expand All @@ -690,19 +692,33 @@ func fakeInitHostFactory(federationName, namespaceName, ip, dnsZoneName, image,
},
},
},
{
Name: "etcddata",
VolumeSource: v1.VolumeSource{
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
ClaimName: pvcName,
},
},
},
},
},
},
},
}
if etcdPersistence == "true" {
Copy link
Contributor

@marun marun Feb 3, 2017 •
edited

Choose a reason for hiding this comment

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

I think having to duplicate code in a test can be a sign of poorly-factored code, a bad test, or both. Will follow up separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please feel free to send a PR refactoring it.

dataVolumeName := "etcddata"
etcdVolume := v1.Volume{
Name: dataVolumeName,
VolumeSource: v1.VolumeSource{
PersistentVolumeClaim: &v1.PersistentVolumeClaimVolumeSource{
ClaimName: pvcName,
},
},
}
etcdVolumeMount := v1.VolumeMount{
Name: dataVolumeName,
MountPath: "/var/etcd",
}

apiserver.Spec.Template.Spec.Volumes = append(apiserver.Spec.Template.Spec.Volumes, etcdVolume)
for i, container := range apiserver.Spec.Template.Spec.Containers {
if container.Name == "etcd" {
apiserver.Spec.Template.Spec.Containers[i].VolumeMounts = append(apiserver.Spec.Template.Spec.Containers[i].VolumeMounts, etcdVolumeMount)
}
}
}

cmName := federationName + "-controller-manager"
cm := v1beta1.Deployment{
Expand Down
1 change: 1 addition & 0 deletions hack/verify-flags/known-flags.txt
View file
Open in desktop
Expand Up @@ -177,6 +177,7 @@ etcd-keyfile
etcd-mutation-timeout
etcd-prefix
etcd-pv-capacity
etcd-persistent-storage
etcd-quorum-read
etcd-server
etcd-servers
Expand Down