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.

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

Run storage provisioner as pod #2137

Merged
merged 4 commits into from
Nov 1, 2017
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
8 changes: 8 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ $(ISO_BUILD_IMAGE): deploy/iso/minikube-iso/Dockerfile
@echo ""
@echo "$(@) successfully built"

storage-provisioner-main: cmd/storage-provisioner/main.go
Copy link
Contributor

Choose a reason for hiding this comment

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

This should technically depend on all the storage-provisioner files.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is not great, but you'll need do add something that computes the dependencies,

STORAGE_PROVISIONER_FILES := GOPATH=$(GOPATH) go list -f '{{join .Deps "\n"}}' k8s.io/minikube/cmd/storage-provisioner | grep k8s.io | GOPATH=$(GOPATH) xargs go list -f '{{ range $$file := .GoFiles }} {{$$.Dir}}/{{$$file}}{{"\n"}}{{end}}'

go build cmd/storage-provisioner/main.go
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to storage-provisioner and can you make this output to our build dir? go build ... -o $(BUILD_DIR)/storage-provisioner


.PHONY: storage-provisioner-image
storage-provisioner-image: storage-provisioner-main
docker build -t $(REGISTRY)/storage-provisioner:$(TAG) -f deploy/storage-provisioner/Dockerfile .
gcloud docker -- push $(REGISTRY)/storage-provisioner:$(TAG)

.PHONY: release-iso
release-iso: minikube_iso checksum
gsutil cp out/minikube.iso gs://$(ISO_BUCKET)/minikube-$(ISO_VERSION).iso
Expand Down
2 changes: 0 additions & 2 deletions cmd/localkube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,4 @@ func SetupServer(s *localkube.LocalkubeServer) {
proxy := s.NewProxyServer()
s.AddServer(proxy)

storageProvisioner := s.NewStorageProvisionerServer()
s.AddServer(storageProvisioner)
}
35 changes: 35 additions & 0 deletions cmd/storage-provisioner/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
Copyright 2016 The Kubernetes Authors All rights reserved.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
"k8s.io/minikube/cmd/localkube/cmd"
"sync"
)

func main() {
localkubeServer := cmd.NewLocalkubeServer()
storageProvisionerServer := localkubeServer.NewStorageProvisionerServer()
Copy link
Contributor

Choose a reason for hiding this comment

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

main should look something like

	flag.Parse()
	if err := localkube.StartStorageProvisioner(); err != nil {
		glog.Exit(err)
	}

Here, instead of creating a new localkube server, change the signature to be func StartStorageProvisioner() error and call that directly.

func StartStorageProvisioner(lk LocalkubeServer) func() error {

Copy link
Author

Choose a reason for hiding this comment

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

SGTM, but we don't need the flag.Parse() in this case, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

glog (the logging library) adds some flags to set verbosity, which would be nice to have on this binary (you might want to run the container command as storage-provisioner --v 10 --logtostderr)

Unfortunately, some of the localkube dependencies also register their flags, so theres some extra crap that gets added, but thats fine.


var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
storageProvisionerServer.Start()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can just call this directly, no need to put it in a waitgroup

Copy link
Author

Choose a reason for hiding this comment

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

So I tried that initially, but it didn't work. I looked it up and I think the main program ends before the server has a chance to start?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think thats due to a few things i mentioned above.
Once you do that, then you can see that its actually just exiting since there are errors

$ ./main --v 10 --logtostderr
F1030 14:08:28.887110  152274 main.go:30] unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined

}()
wg.Wait()
}
29 changes: 29 additions & 0 deletions deploy/addons/storage-provisioner/storage-provisioner.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Copyright 2016 The Kubernetes Authors All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: v1
kind: Pod
metadata:
name: storage-provisioner
namespace: kube-system
labels:
integration-test: storage-provisioner
addonmanager.kubernetes.io/mode: EnsureExists
kubernetes.io/minikube-addons: storage-provisioner
spec:
hostNetwork: true
containers:
- name: storage-provisioner
image: gcr.io/k8s-minikube/storage-provisioner:v1.8.0
imagePullPolicy: IfNotPresent
18 changes: 18 additions & 0 deletions deploy/storage-provisioner/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Copyright 2016 The Kubernetes Authors All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

FROM scratch
COPY main main
CMD ["/main"]

7 changes: 3 additions & 4 deletions pkg/localkube/storage_provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ import (
"k8s.io/apimachinery/pkg/util/uuid"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/minikube/pkg/util"
restclient "k8s.io/client-go/rest"
)

const provisionerName = "k8s.io/minikube-hostpath"
Expand Down Expand Up @@ -115,9 +114,8 @@ func (lk LocalkubeServer) NewStorageProvisionerServer() Server {
}

func StartStorageProvisioner(lk LocalkubeServer) func() error {

return func() error {
config, err := clientcmd.BuildConfigFromFlags("", util.DefaultKubeConfigPath)
config, err := restclient.InClusterConfig()
if err != nil {
return err
}
Expand All @@ -141,6 +139,7 @@ func StartStorageProvisioner(lk LocalkubeServer) func() error {
// PVs
pc := controller.NewProvisionController(clientset, provisionerName, hostPathProvisioner, serverVersion.GitVersion)

fmt.Println("wait never stop")
pc.Run(wait.NeverStop)
return nil
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/minikube/assets/addons.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ var Addons = map[string]*Addon{
"storageclass.yaml",
"0640"),
}, true, "default-storageclass"),
"storage-provisioner": NewAddon([]*BinDataAsset{
NewBinDataAsset(
"deploy/addons/storage-provisioner/storage-provisioner.yaml",
constants.AddonsPath,
"storage-provisioner.yaml",
"0640"),
}, true, "storage-provisioner"),
"coredns": NewAddon([]*BinDataAsset{
NewBinDataAsset(
"deploy/addons/coredns/coreDNS-controller.yaml",
Expand Down
6 changes: 6 additions & 0 deletions pkg/minikube/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ var LocalkubeCachedImages = []string{

// Pause
"gcr.io/google_containers/pause-amd64:3.0",

//Storage Provisioner
"gcr.io/k8s-minikube/storage-provisioner:v1.8.0",
}

func GetKubeadmCachedImages(version string) []string {
Expand All @@ -206,6 +209,9 @@ func GetKubeadmCachedImages(version string) []string {
"gcr.io/google_containers/kube-scheduler-amd64:" + version,
"gcr.io/google_containers/kube-controller-manager-amd64:" + version,
"gcr.io/google_containers/kube-apiserver-amd64:" + version,

//Storage Provisioner
"gcr.io/k8s-minikube/storage-provisioner:v1.8.0",
}
}

Expand Down
6 changes: 1 addition & 5 deletions test/integration/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ func TestFunctional(t *testing.T) {
t.Run("Addons", testAddons)
t.Run("Dashboard", testDashboard)
t.Run("ServicesList", testServicesList)

// Don't run this test on kubeadm bootstrapper for now.
if !strings.Contains(*args, "--bootstrapper=kubeadm") {
t.Run("Provisioning", testProvisioning)
}
t.Run("Provisioning", testProvisioning)

if !strings.Contains(minikubeRunner.StartArgs, "--vm-driver=none") {
t.Run("EnvVars", testClusterEnv)
Expand Down
22 changes: 22 additions & 0 deletions test/integration/pv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ package integration

import (
"fmt"
"github.com/pkg/errors"
"path/filepath"
"testing"
"time"

"k8s.io/apimachinery/pkg/labels"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/storage"
commonutil "k8s.io/minikube/pkg/util"
"k8s.io/minikube/test/integration/util"
)

Expand Down Expand Up @@ -58,6 +61,25 @@ func testProvisioning(t *testing.T) {
t.Fatalf("No default storage class: %s", err)
}

// Check that the storage provisioner pod is running

checkPodRunning := func() error {
client, err := commonutil.GetClient()
if err != nil {
return errors.Wrap(err, "getting kubernetes client")
}
selector := labels.SelectorFromSet(labels.Set(map[string]string{"integration-test": "storage-provisioner"}))

if err := commonutil.WaitForPodsWithLabelRunning(client, "kube-system", selector); err != nil {
return err
}
return nil
}

if err := checkPodRunning(); err != nil {
t.Fatal("Check storage-provisioner pod running failed with error: ", err)
}

// Now create the PVC
pvcPath := filepath.Join(*testdataDir, "pvc.yaml")
if _, err := kubectlRunner.RunCommand([]string{"create", "-f", pvcPath}); err != nil {
Expand Down