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

operator: fix bugs on reading configuration from config-map #10520

Merged
merged 5 commits into from
Mar 12, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 1 addition & 5 deletions daemon/daemon_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,7 @@ func init() {
return
}

if viper.GetBool("version") {
fmt.Printf("Cilium %s\n", version.Version)
os.Exit(0)
}
cobra.OnInitialize(option.InitConfig("ciliumd"))
cobra.OnInitialize(option.InitConfig("Cilium", "ciliumd"))

// Reset the help function to also exit, as we block elsewhere in interrupts
// and would not exit when called with -h.
Expand Down
8 changes: 4 additions & 4 deletions operator/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ import (
"fmt"
"time"

"github.com/cilium/cilium/pkg/defaults"
"github.com/cilium/cilium/pkg/option"

"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/klog"

"github.com/cilium/cilium/pkg/defaults"
"github.com/cilium/cilium/pkg/option"
)

func init() {
cobra.OnInitialize(initConfig)
cobra.OnInitialize(option.InitConfig("Cilium-Operator", "cilium-operator"))

flags := rootCmd.Flags()

Expand Down
9 changes: 0 additions & 9 deletions operator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,6 @@ func main() {
}
}

// initConfig reads in config file and ENV variables if set.
func initConfig() {
if viper.GetBool("version") {
fmt.Printf("Cilium-Operator %s\n", version.Version)
os.Exit(0)
}
cobra.OnInitialize(option.InitConfig("cilium-operator"))
}

func kvstoreEnabled() bool {
if option.Config.KVStore == "" {
return false
Expand Down
24 changes: 17 additions & 7 deletions pkg/option/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/cilium/cilium/pkg/logging"
"github.com/cilium/cilium/pkg/logging/logfields"
"github.com/cilium/cilium/pkg/metrics"
"github.com/cilium/cilium/pkg/version"

"github.com/prometheus/client_golang/prometheus"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -2268,7 +2269,9 @@ func (c *DaemonConfig) populateNodePortRange() error {
return errors.New("NodePort range min port must be smaller than max port")
}
case 0:
log.Warning("NodePort range was set but is empty.")
if viper.IsSet(NodePortRange) {
log.Warning("NodePort range was set but is empty.")
}
default:
return fmt.Errorf("Unable to parse min/max port value for NodePort range: %s", NodePortRange)
}
Expand Down Expand Up @@ -2305,11 +2308,13 @@ func (c *DaemonConfig) populateHostServicesProtos() error {
func sanitizeIntParam(paramName string, paramDefault int) int {
intParam := viper.GetInt(paramName)
if intParam <= 0 {
log.WithFields(
logrus.Fields{
"parameter": paramName,
"defaultValue": paramDefault,
}).Warning("user-provided parameter had value <= 0 , which is invalid ; setting to default")
if !viper.IsSet(paramName) {
log.WithFields(
logrus.Fields{
"parameter": paramName,
"defaultValue": paramDefault,
}).Warning("user-provided parameter had value <= 0 , which is invalid ; setting to default")
}
return paramDefault
}
return intParam
Expand Down Expand Up @@ -2340,8 +2345,13 @@ func getHostDevice() string {
}

// InitConfig reads in config file and ENV variables if set.
func InitConfig(configName string) func() {
func InitConfig(programName, configName string) func() {
return func() {
if viper.GetBool("version") {
fmt.Printf("%s %s\n", programName, version.Version)
os.Exit(0)
}

if viper.GetString(CMDRef) != "" {
return
}
Expand Down
46 changes: 45 additions & 1 deletion test/helpers/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1493,6 +1493,43 @@ func (kub *Kubectl) GetPublicIface() (string, error) {
return kub.getIfaceByIPAddr(K8s1, ipAddr)
}

func (kub *Kubectl) DeleteCiliumOperator() error {

Choose a reason for hiding this comment

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

exported method Kubectl.DeleteCiliumOperator should have comment or be unexported

// Do not assert on success in AfterEach intentionally to avoid
// incomplete teardown.

_ = kub.DeleteResource("deployment", fmt.Sprintf("-n %s cilium-operator", GetCiliumNamespace(GetCurrentIntegration())))
return kub.waitToDeleteCiliumOperator()
}

func (kub *Kubectl) waitToDeleteCiliumOperator() error {
var (
pods []string
err error
)

ctx, cancel := context.WithTimeout(context.Background(), HelperTimeout)
defer cancel()

status := 1
for status > 0 {

select {
case <-ctx.Done():
return fmt.Errorf("timed out waiting to delete Cilium Operator: pods still remaining: %s", pods)
Copy link
Member

Choose a reason for hiding this comment

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

pods will be uninitialized here on first run, probably won't ever happen, but can we move whole select to the end of the loop body? I assume we want to run through the loop body at least once.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nebril I'm going to fix this in another PR as this PR is blocking some of my work.

default:
}

pods, err = kub.GetPodNamesContext(ctx, GetCiliumNamespace(GetCurrentIntegration()), "io.cilium/app=cilium")
status = len(pods)
kub.Logger().Infof("Cilium Operator pods terminating '%d' err='%v' pods='%v'", status, err, pods)
if status == 0 {
return nil
}
time.Sleep(1 * time.Second)
}
return nil
}

func (kub *Kubectl) DeleteCiliumDS() error {
// Do not assert on success in AfterEach intentionally to avoid
// incomplete teardown.
Expand Down Expand Up @@ -1557,6 +1594,13 @@ func (kub *Kubectl) CiliumInstall(filename string, options map[string]string) er
return err
}

// Remove cilium operator to ensure that new instances of cilium-operator are started
// for the newly generated ConfigMap. Otherwise, the CM changes will stay
// inactive until each cilium-operator has been restarted.
if err := kub.DeleteCiliumOperator(); err != nil {
return err
}

res := kub.Apply(ApplyOptions{FilePath: filename, Force: true, Namespace: GetCiliumNamespace(GetCurrentIntegration())})
if !res.WasSuccessful() {
return res.GetErr("Unable to apply YAML")
Expand Down Expand Up @@ -2757,7 +2801,7 @@ func (kub *Kubectl) ciliumHealthPreFlightCheck() error {
if len(ciliumPods) != len(nodeCount) {
return fmt.Errorf(
"cilium-agent '%s': Only %d/%d nodes appeared in cilium-health status. nodes = '%+v'",
pod, len(nodeCount), len(ciliumPods), nodeCount)
pod, len(ciliumPods), len(nodeCount), nodeCount)
}

healthStatus, err := status.Filter(statusFilter)
Expand Down