Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ VERSION_OVERRIDE:=$(shell git describe --abbrev=8 --dirty --always)
HASH:=$(shell git rev-parse --verify 'HEAD^{commit}')
BUILD_DATE:=$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')
GLDFLAGS=-X $(REPO)/pkg/version.versionFromGit=$(VERSION_OVERRIDE) -X $(REPO)/pkg/version.commitFromGit=$(HASH) -X $(REPO)/pkg/version.buildDate=$(BUILD_DATE)
GOFALGS=
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was giving a warning obviously not needed cleaned up here.


all: build
.PHONY: all
Expand All @@ -18,7 +17,7 @@ build: bin/cluster-etcd-operator

bin/cluster-etcd-operator: $(GOFILES)
@echo Building $@
@go build $(GOFLAGS) -ldflags "$(GLDFLAGS)" -o $(ROOT_DIR)/$@ github.com/openshift/cluster-etcd-operator/cmd/cluster-etcd-operator
@go build -ldflags "$(GLDFLAGS)" -o $(ROOT_DIR)/$@ github.com/openshift/cluster-etcd-operator/cmd/cluster-etcd-operator

test-unit:
@go test -v ./...
Expand Down
2 changes: 2 additions & 0 deletions cmd/cluster-etcd-operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/openshift/cluster-etcd-operator/pkg/cmd/staticpodcontroller"
"github.com/openshift/cluster-etcd-operator/pkg/cmd/staticsynccontroller"
"github.com/openshift/cluster-etcd-operator/pkg/cmd/waitforceo"
"github.com/openshift/cluster-etcd-operator/pkg/cmd/waitforkube"
"github.com/spf13/cobra"
"github.com/spf13/pflag"

Expand Down Expand Up @@ -52,6 +53,7 @@ func NewSSCSCommand() *cobra.Command {
cmd.AddCommand(staticpodcontroller.NewStaticPodCommand(os.Stderr))
cmd.AddCommand(mount.NewMountCommand(os.Stderr))
cmd.AddCommand(waitforceo.NewWaitForCeoCommand(os.Stderr))
cmd.AddCommand(waitforkube.NewWaitForKubeCommand(os.Stderr))

return cmd
}
68 changes: 68 additions & 0 deletions pkg/cmd/waitforkube/waitforkube.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package waitforkube

import (
"fmt"
"io"
"os"
"time"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog"
)

const (
retryDuration = 10 * time.Second
saTokenPath = "/var/run/secrets/kubernetes.io/serviceaccount/token"
)

type waitForKubeOpts struct {
errOut io.Writer
}

// NewWaitForKubeCommand waits for kube to come up before continuing to start the rest of the containers.
func NewWaitForKubeCommand(errOut io.Writer) *cobra.Command {
waitForKubeOpts := &waitForKubeOpts{
errOut: errOut,
}
cmd := &cobra.Command{
Use: "wait-for-kube",
Short: "wait for kube service account to exist",
Long: "This command makes sure that the kube is available before starting the rest of the containers in the pod.",
Run: func(cmd *cobra.Command, args []string) {
must := func(fn func() error) {
if err := fn(); err != nil {
if cmd.HasParent() {
klog.Fatal(err)
fmt.Fprint(waitForKubeOpts.errOut, err.Error())
}
}
}
must(waitForKubeOpts.Run)
},
}

return cmd
}

func (w *waitForKubeOpts) Run() error {
wait.PollInfinite(retryDuration, func() (bool, error) {
if _, err := os.Stat(saTokenPath); os.IsNotExist(err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is it sufficient enough to check that the env variables are populated with this?

Should we have an equivalent of inCluster function here and check for env variable to be safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is it sufficient enough to check that the env variables are populated with this?

If the SA token has synced that means kube scheduled staticsync so yes.

Should we have an equivalent of inCluster function here and check for env variable to be safe?

Where? we are waiting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be clear static sync is a daemonset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

our container will not change its ENV unless it restarts right?

Copy link
Contributor

@alaypatel07 alaypatel07 Dec 9, 2019

Choose a reason for hiding this comment

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

yes ideally it will not change, but want to be sure with something like

if !inCluster {
    return fmt.Errorf("kube env not populated")
}

Where inCluster is

func inCluster() bool {
	if os.Getenv("KUBERNETES_SERVICE_HOST") == "" || os.Getenv("KUBERNETES_SERVICE_PORT") == "" {
		return false
	}
	return true
}

We can either do it after the wait loop, so the initContainer will crashlooping until the env is populated.

if the env is not populated, it is not a bug against us

klog.Infof("waiting for kube service account resources to sync: %v", err)
return false, nil
}
return true, nil
})
if !inCluster() {
return fmt.Errorf("kube env not populated")
}
return nil
}

//TODO: add to util
func inCluster() bool {
if os.Getenv("KUBERNETES_SERVICE_HOST") == "" || os.Getenv("KUBERNETES_SERVICE_PORT") == "" {
return false
}
return true
}