Skip to content

Commit

Permalink
refactor. (#33759)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bianpengyuan
bianpengyuan authored Jul 1, 2021
1 parent 4ee3b5c commit a3a734a
Show file tree
Hide file tree
Showing 35 changed files with 311 additions and 417 deletions.
2 changes: 1 addition & 1 deletion cni/cmd/install-cni/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"os/signal"
"syscall"

"istio.io/istio/cni/pkg/install-cni/cmd"
"istio.io/istio/cni/pkg/cmd"
"istio.io/pkg/log"
)

Expand Down
64 changes: 53 additions & 11 deletions cni/pkg/install-cni/cmd/root.go → cni/pkg/cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,12 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/viper"

"istio.io/istio/cni/pkg/install-cni/pkg/config"
"istio.io/istio/cni/pkg/install-cni/pkg/constants"
"istio.io/istio/cni/pkg/install-cni/pkg/install"
"istio.io/istio/cni/pkg/config"
"istio.io/istio/cni/pkg/constants"
"istio.io/istio/cni/pkg/install"
"istio.io/istio/cni/pkg/monitoring"
"istio.io/istio/cni/pkg/repair"
iptables "istio.io/istio/tools/istio-iptables/pkg/constants"
"istio.io/pkg/log"
)

Expand All @@ -43,16 +44,17 @@ var rootCmd = &cobra.Command{
if cfg, err = constructConfig(); err != nil {
return
}
log.Infof("install cni with configuration: \n%+v", cfg)
log.Infof("CNI install configuration: \n%+v", cfg.InstallConfig)
log.Infof("CNI race repair configuration: \n%+v", cfg.RepairConfig)

// Start metrics server
monitoring.SetupMonitoring(":15014", "/metrics", ctx.Done())
monitoring.SetupMonitoring(":"+constants.MonitoringPort, "/metrics", ctx.Done())

isReady := install.StartServer()

installer := install.NewInstaller(cfg, isReady)
installer := install.NewInstaller(&cfg.InstallConfig, isReady)

repair.StartRepair(ctx)
repair.StartRepair(ctx, &cfg.RepairConfig)

if err = installer.Run(ctx); err != nil {
if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
Expand Down Expand Up @@ -97,6 +99,30 @@ func init() {
registerBooleanParameter(constants.SkipTLSVerify, false, "Whether to use insecure TLS in kubeconfig file")
registerBooleanParameter(constants.UpdateCNIBinaries, true, "Update binaries")
registerStringArrayParameter(constants.SkipCNIBinaries, []string{}, "Binaries that should not be installed")

// Repair
registerBooleanParameter(constants.RepairEnabled, true, "Whether to enable race condition repair or not")
registerBooleanParameter(constants.RepairDeletePods, false, "Controller will delete pods")
registerBooleanParameter(constants.RepairLabelPods, false, "Controller will label pods")
registerBooleanParameter(constants.RepairRunAsDaemon, false, "Controller will run in a loop")
registerStringParameter(constants.RepairLabelKey, "cni.istio.io/uninitialized",
"The key portion of the label which will be set by the reconciler if --label-pods is true")
registerStringParameter(constants.RepairLabelValue, "true",
"The value portion of the label which will be set by the reconciler if --label-pods is true")
registerStringParameter(constants.RepairNodeName, "", "The name of the managed node (will manage all nodes if unset)")
registerStringParameter(constants.RepairSidecarAnnotation, "sidecar.istio.io/status",
"An annotation key that indicates this pod contains an istio sidecar. All pods without this annotation will be ignored."+
"The value of the annotation is ignored.")
registerStringParameter(constants.RepairInitContainerName, "istio-validation",
"The name of the istio init container (will crash-loop if CNI is not configured for the pod)")
registerStringParameter(constants.RepairInitTerminationMsg, "",
"The expected termination message for the init container when crash-looping because of CNI misconfiguration")
registerIntegerParameter(constants.RepairInitExitCode, iptables.ValidationErrorCode,
"Expected exit code for the init container when crash-looping because of CNI misconfiguration")
registerStringParameter(constants.RepairLabelSelectors, "",
"A set of label selectors in label=value format that will be added to the pod list filters")
registerStringParameter(constants.RepairFieldSelectors, "",
"A set of field selectors in label=value format that will be added to the pod list filters")
}

func registerStringParameter(name, value, usage string) {
Expand Down Expand Up @@ -127,7 +153,7 @@ func bindViper(name string) {
}

func constructConfig() (*config.Config, error) {
cfg := &config.Config{
installCfg := config.InstallConfig{
CNINetDir: viper.GetString(constants.CNINetDir),
MountedCNINetDir: viper.GetString(constants.MountedCNINetDir),
CNIConfName: viper.GetString(constants.CNIConfName),
Expand All @@ -152,13 +178,29 @@ func constructConfig() (*config.Config, error) {
SkipCNIBinaries: viper.GetStringSlice(constants.SkipCNIBinaries),
}

if len(cfg.K8sNodeName) == 0 {
if len(installCfg.K8sNodeName) == 0 {
var err error
cfg.K8sNodeName, err = os.Hostname()
installCfg.K8sNodeName, err = os.Hostname()
if err != nil {
return nil, err
}
}

return cfg, nil
repairCfg := config.RepairConfig{
Enabled: viper.GetBool(constants.RepairEnabled),
DeletePods: viper.GetBool(constants.RepairDeletePods),
LabelPods: viper.GetBool(constants.RepairLabelPods),
RunAsDaemon: viper.GetBool(constants.RepairRunAsDaemon),
LabelKey: viper.GetString(constants.RepairLabelKey),
LabelValue: viper.GetString(constants.RepairLabelValue),
NodeName: viper.GetString(constants.RepairNodeName),
SidecarAnnotation: viper.GetString(constants.RepairSidecarAnnotation),
InitContainerName: viper.GetString(constants.RepairInitContainerName),
InitTerminationMsg: viper.GetString(constants.RepairInitTerminationMsg),
InitExitCode: viper.GetInt(constants.RepairInitExitCode),
LabelSelectors: viper.GetString(constants.RepairLabelSelectors),
FieldSelectors: viper.GetString(constants.RepairFieldSelectors),
}

return &config.Config{InstallConfig: installCfg, RepairConfig: repairCfg}, nil
}
61 changes: 59 additions & 2 deletions cni/pkg/install-cni/pkg/config/config.go → cni/pkg/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,13 @@ import (
"strings"
)

// Config struct defines the Istio CNI installation options
type Config struct {
InstallConfig InstallConfig
RepairConfig RepairConfig
}

// InstallConfig struct defines the Istio CNI installation options
type InstallConfig struct {
// Location of the CNI config files in the host's filesystem
CNINetDir string
// Location of the CNI config files in the container's filesystem (mount location of the CNINetDir)
Expand Down Expand Up @@ -66,7 +71,41 @@ type Config struct {
SkipCNIBinaries []string
}

func (c *Config) String() string {
// RepairConfig struct defines the Istio CNI race repair configuration
type RepairConfig struct {
// Whether to enable CNI race repair
Enabled bool

// Whether to run CNI as a DaemonSet (i.e. continuously via k8s watch),
// or just one-off
RunAsDaemon bool

// The node name that the CNI DaemonSet runs on
NodeName string

// Key and value for broken pod label
LabelKey string
LabelValue string

// Whether to fix race condition by delete broken pods
DeletePods bool

// Whether to label broken pods
LabelPods bool

// Filters for race repair, including name of sidecar annotation, name of init container,
// init container termination message and exit code.
SidecarAnnotation string
InitContainerName string
InitTerminationMsg string
InitExitCode int

// Label and field selectors to select pods managed by race repair.
LabelSelectors string
FieldSelectors string
}

func (c InstallConfig) String() string {
var b strings.Builder
b.WriteString("CNINetDir: " + c.CNINetDir + "\n")
b.WriteString("MountedCNINetDir: " + c.MountedCNINetDir + "\n")
Expand All @@ -89,3 +128,21 @@ func (c *Config) String() string {
b.WriteString("SkipCNIBinaries: " + fmt.Sprint(c.SkipCNIBinaries) + "\n")
return b.String()
}

func (c RepairConfig) String() string {
var b strings.Builder
b.WriteString("Enabled: " + fmt.Sprint(c.Enabled) + "\n")
b.WriteString("RunAsDaemon: " + fmt.Sprint(c.RunAsDaemon) + "\n")
b.WriteString("NodeName: " + c.NodeName + "\n")
b.WriteString("LabelKey: " + c.LabelKey + "\n")
b.WriteString("LabelValue: " + c.LabelValue + "\n")
b.WriteString("DeletePods: " + fmt.Sprint(c.DeletePods) + "\n")
b.WriteString("LabelPods: " + fmt.Sprint(c.LabelPods) + "\n")
b.WriteString("SidecarAnnotation: " + c.SidecarAnnotation + "\n")
b.WriteString("InitContainerName: " + c.InitContainerName + "\n")
b.WriteString("InitTerminationMsg: " + c.InitTerminationMsg + "\n")
b.WriteString("InitExitCode: " + fmt.Sprint(c.InitExitCode) + "\n")
b.WriteString("LabelSelectors: " + c.LabelSelectors + "\n")
b.WriteString("FieldSelectors: " + c.FieldSelectors + "\n")
return b.String()
}
17 changes: 17 additions & 0 deletions ...kg/install-cni/pkg/constants/constants.go → cni/pkg/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package constants

// Command line arguments
const (
// Install
MountedCNINetDir = "mounted-cni-net-dir"
CNINetDir = "cni-net-dir"
CNIConfName = "cni-conf-name"
Expand All @@ -29,6 +30,21 @@ const (
SkipTLSVerify = "skip-tls-verify"
SkipCNIBinaries = "skip-cni-binaries"
UpdateCNIBinaries = "update-cni-binaries"

// Repair
RepairEnabled = "repair-enabled"
RepairDeletePods = "repair-delete-pods"
RepairLabelPods = "repair-label-pods"
RepairRunAsDaemon = "repair-run-as-daemon"
RepairLabelKey = "repair-broken-pod-label-key"
RepairLabelValue = "repair-broken-pod-label-value"
RepairNodeName = "repair-node-name"
RepairSidecarAnnotation = "repair-sidecar-annotation"
RepairInitContainerName = "repair-init-container-name"
RepairInitTerminationMsg = "repair-init-container-termination-message"
RepairInitExitCode = "repair-init-container-exit-code"
RepairLabelSelectors = "repair-label-selectors"
RepairFieldSelectors = "repair-field-selectors"
)

// Internal constants
Expand All @@ -43,4 +59,5 @@ var (
LivenessEndpoint = "/healthz"
ReadinessEndpoint = "/readyz"
Port = "8000"
MonitoringPort = "15014"
)
0 cni/pkg/install-cni/pkg/install/binaries.go → cni/pkg/install/binaries.go
View file
File renamed without changes.
0 .../install-cni/pkg/install/binaries_test.go → cni/pkg/install/binaries_test.go
View file
File renamed without changes.
12 changes: 6 additions & 6 deletions cni/pkg/install-cni/pkg/install/cniconfig.go → cni/pkg/install/cniconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (
"github.com/containernetworking/cni/libcni"
"github.com/pkg/errors"

"istio.io/istio/cni/pkg/install-cni/pkg/config"
"istio.io/istio/cni/pkg/install-cni/pkg/util"
"istio.io/istio/cni/pkg/config"
"istio.io/istio/cni/pkg/util"
"istio.io/istio/pkg/file"
"istio.io/pkg/log"
)
Expand All @@ -53,22 +53,22 @@ type cniConfigVars struct {
k8sNodeName string
}

func getPluginConfig(cfg *config.Config) pluginConfig {
func getPluginConfig(cfg *config.InstallConfig) pluginConfig {
return pluginConfig{
mountedCNINetDir: cfg.MountedCNINetDir,
cniConfName: cfg.CNIConfName,
chainedCNIPlugin: cfg.ChainedCNIPlugin,
}
}

func getCNIConfigTemplate(cfg *config.Config) cniConfigTemplate {
func getCNIConfigTemplate(cfg *config.InstallConfig) cniConfigTemplate {
return cniConfigTemplate{
cniNetworkConfigFile: cfg.CNINetworkConfigFile,
cniNetworkConfig: cfg.CNINetworkConfig,
}
}

func getCNIConfigVars(cfg *config.Config) cniConfigVars {
func getCNIConfigVars(cfg *config.InstallConfig) cniConfigVars {
return cniConfigVars{
cniNetDir: cfg.CNINetDir,
kubeconfigFilename: cfg.KubeconfigFilename,
Expand All @@ -79,7 +79,7 @@ func getCNIConfigVars(cfg *config.Config) cniConfigVars {
}
}

func createCNIConfigFile(ctx context.Context, cfg *config.Config, saToken string) (string, error) {
func createCNIConfigFile(ctx context.Context, cfg *config.InstallConfig, saToken string) (string, error) {
cniConfig, err := readCNIConfigTemplate(getCNIConfigTemplate(cfg))
if err != nil {
return "", err
Expand Down
8 changes: 4 additions & 4 deletions ...install-cni/pkg/install/cniconfig_test.go → cni/pkg/install/cniconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

"github.com/stretchr/testify/assert"

"istio.io/istio/cni/pkg/install-cni/pkg/config"
"istio.io/istio/cni/pkg/config"
testutils "istio.io/istio/pilot/test/util"
"istio.io/istio/pkg/file"
)
Expand Down Expand Up @@ -471,22 +471,22 @@ func TestCreateCNIConfigFile(t *testing.T) {
}

for i, c := range cases {
cfgFile := config.Config{
cfgFile := config.InstallConfig{
CNIConfName: c.specifiedConfName,
ChainedCNIPlugin: c.chainedCNIPlugin,
CNINetworkConfigFile: cniNetworkConfigFile,
LogLevel: "debug",
KubeconfigFilename: kubeconfigFilename,
}

cfg := config.Config{
cfg := config.InstallConfig{
CNIConfName: c.specifiedConfName,
ChainedCNIPlugin: c.chainedCNIPlugin,
CNINetworkConfig: cniNetworkConfig,
LogLevel: "debug",
KubeconfigFilename: kubeconfigFilename,
}
test := func(cfg config.Config) func(t *testing.T) {
test := func(cfg config.InstallConfig) func(t *testing.T) {
return func(t *testing.T) {
// Create temp directory for files
tempDir, err := ioutil.TempDir("", fmt.Sprintf("test-case-%d-", i))
Expand Down
14 changes: 7 additions & 7 deletions cni/pkg/install-cni/pkg/install/install.go → cni/pkg/install/install.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,23 @@ import (

"github.com/pkg/errors"

"istio.io/istio/cni/pkg/install-cni/pkg/config"
"istio.io/istio/cni/pkg/install-cni/pkg/constants"
"istio.io/istio/cni/pkg/install-cni/pkg/util"
"istio.io/istio/cni/pkg/config"
"istio.io/istio/cni/pkg/constants"
"istio.io/istio/cni/pkg/util"
"istio.io/istio/pkg/file"
"istio.io/pkg/log"
)

type Installer struct {
cfg *config.Config
cfg *config.InstallConfig
isReady *atomic.Value
saToken string
kubeconfigFilepath string
cniConfigFilepath string
}

// NewInstaller returns an instance of Installer with the given config
func NewInstaller(cfg *config.Config, isReady *atomic.Value) *Installer {
func NewInstaller(cfg *config.InstallConfig, isReady *atomic.Value) *Installer {
return &Installer{
cfg: cfg,
isReady: isReady,
Expand Down Expand Up @@ -161,7 +161,7 @@ func readServiceAccountToken() (string, error) {
// sleepCheckInstall verifies the configuration then blocks until an invalid configuration is detected, and return nil.
// If an error occurs or context is canceled, the function will return the error.
// Returning from this function will set the pod to "NotReady".
func sleepCheckInstall(ctx context.Context, cfg *config.Config, cniConfigFilepath string, isReady *atomic.Value) error {
func sleepCheckInstall(ctx context.Context, cfg *config.InstallConfig, cniConfigFilepath string, isReady *atomic.Value) error {
// Create file watcher before checking for installation
// so that no file modifications are missed while and after checking
watcher, fileModified, errChan, err := util.CreateFileWatcher(cfg.MountedCNINetDir)
Expand Down Expand Up @@ -199,7 +199,7 @@ func sleepCheckInstall(ctx context.Context, cfg *config.Config, cniConfigFilepat
}

// checkInstall returns an error if an invalid CNI configuration is detected
func checkInstall(cfg *config.Config, cniConfigFilepath string) error {
func checkInstall(cfg *config.InstallConfig, cniConfigFilepath string) error {
defaultCNIConfigFilename, err := getDefaultCNINetwork(cfg.MountedCNINetDir)
if err != nil {
return err
Expand Down
Loading

0 comments on commit a3a734a

Please sign in to comment.