Add testing for Start function, make addon enablement testable

pull/6440/head
Thomas Stromberg 2020-01-31 12:19:25 -08:00
parent ce68dd19b6
commit 78ed12265d
8 changed files with 131 additions and 98 deletions

View File

@ -102,7 +102,7 @@ var printAddonsList = func() {
for _, addonName := range addonNames {
addonBundle := assets.Addons[addonName]
addonStatus, err := addonBundle.IsEnabled()
addonStatus, err := addonBundle.IsEnabled(pName)
if err != nil {
out.WarningT("Unable to get addon status for {{.name}}: {{.error}}", out.V{"name": addonName, "error": err})
}
@ -134,7 +134,7 @@ var printAddonsJSON = func() {
for _, addonName := range addonNames {
addonBundle := assets.Addons[addonName]
addonStatus, err := addonBundle.IsEnabled()
addonStatus, err := addonBundle.IsEnabled(pName)
if err != nil {
glog.Errorf("Unable to get addon status for %s: %v", addonName, err)
continue

View File

@ -24,12 +24,14 @@ import (
"github.com/pkg/browser"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/assets"
"k8s.io/minikube/pkg/minikube/cluster"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/machine"
"k8s.io/minikube/pkg/minikube/out"
"k8s.io/minikube/pkg/minikube/service"
pkg_config "k8s.io/minikube/pkg/minikube/config"
)
var (
@ -66,7 +68,8 @@ var addonsOpenCmd = &cobra.Command{
}
defer api.Close()
if !cluster.IsMinikubeRunning(api) {
profileName := viper.GetString(pkg_config.MachineProfile)
if !cluster.IsHostRunning(api, profileName) {
os.Exit(1)
}
addon, ok := assets.Addons[addonName] // validate addon input
@ -75,7 +78,7 @@ var addonsOpenCmd = &cobra.Command{
To see the list of available addons run:
minikube addons list`, out.V{"name": addonName})
}
ok, err = addon.IsEnabled()
ok, err = addon.IsEnabled(profileName)
if err != nil {
exit.WithError("IsEnabled failed", err)
}

View File

@ -36,7 +36,6 @@ import (
pkgaddons "k8s.io/minikube/pkg/addons"
"k8s.io/minikube/pkg/minikube/assets"
"k8s.io/minikube/pkg/minikube/cluster"
"k8s.io/minikube/pkg/minikube/config"
pkg_config "k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/machine"
@ -103,18 +102,18 @@ var dashboardCmd = &cobra.Command{
exit.WithCodeT(exit.NoInput, "kubectl not found in PATH, but is required for the dashboard. Installation guide: https://kubernetes.io/docs/tasks/tools/install-kubectl/")
}
if !cluster.IsMinikubeRunning(api) {
if !cluster.IsHostRunning(api, profileName) {
os.Exit(1)
}
// Check dashboard status before enabling it
dashboardAddon := assets.Addons["dashboard"]
dashboardStatus, _ := dashboardAddon.IsEnabled()
dashboardStatus, _ := dashboardAddon.IsEnabled(profileName)
if !dashboardStatus {
// Send status messages to stderr for folks re-using this output.
out.ErrT(out.Enabling, "Enabling dashboard ...")
// Enable the dashboard add-on
err = pkgaddons.Set("dashboard", "true", viper.GetString(config.MachineProfile))
err = pkgaddons.Set("dashboard", "true", profileName)
if err != nil {
exit.WithError("Unable to enable dashboard", err)
}

View File

@ -25,8 +25,10 @@ import (
"github.com/golang/glog"
"github.com/pkg/browser"
"github.com/spf13/cobra"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/cluster"
pkg_config "k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/exit"
"k8s.io/minikube/pkg/minikube/machine"
"k8s.io/minikube/pkg/minikube/out"
@ -71,7 +73,8 @@ var serviceCmd = &cobra.Command{
}
defer api.Close()
if !cluster.IsMinikubeRunning(api) {
profileName := viper.GetString(pkg_config.MachineProfile)
if !cluster.IsHostRunning(api, profileName) {
os.Exit(1)
}

View File

@ -44,7 +44,6 @@ const defaultStorageClassProvisioner = "standard"
// Set sets a value
func Set(name, value, profile string) error {
glog.Infof("Setting %s=%s in profile %q", name, value, profile)
a, valid := isAddonValid(name)
if !valid {
return errors.Errorf("%s is not a valid addon", name)
@ -70,6 +69,7 @@ func Set(name, value, profile string) error {
return errors.Wrap(err, "running callbacks")
}
glog.Infof("Writing new config for %q ...", profile)
// Write the value
return config.Write(profile, c)
}
@ -112,7 +112,7 @@ func enableOrDisableAddon(name, val, profile string) error {
addon := assets.Addons[name]
// check addon status before enabling/disabling it
alreadySet, err := isAddonAlreadySet(addon, enable)
alreadySet, err := isAddonAlreadySet(addon, enable, profile)
if err != nil {
out.ErrT(out.Conflict, "{{.error}}", out.V{"error": err})
return err
@ -139,21 +139,15 @@ func enableOrDisableAddon(name, val, profile string) error {
}
defer api.Close()
//if minikube is not running, we return and simply update the value in the addon
//config and rewrite the file
if !cluster.IsMinikubeRunning(api) {
glog.Warningf("minikube is not running, writing to disk and doing nothing")
return nil
}
cfg, err := config.Load(profile)
if err != nil && !os.IsNotExist(err) {
exit.WithCodeT(exit.Data, "Unable to load config: {{.error}}", out.V{"error": err})
}
host, err := cluster.CheckIfHostExistsAndLoad(api, cfg.Name)
if err != nil {
return errors.Wrap(err, "getting host")
host, err := cluster.CheckIfHostExistsAndLoad(api, profile)
if err != nil || !cluster.IsHostRunning(api, profile) {
glog.Warningf("%q is not running, writing to %s=%v disk and skipping the rest (err=%v)", profile, addon.Name(), enable, err)
return nil
}
cmd, err := machine.CommandRunner(host)
@ -165,11 +159,10 @@ func enableOrDisableAddon(name, val, profile string) error {
return enableOrDisableAddonInternal(addon, cmd, data, enable, profile)
}
func isAddonAlreadySet(addon *assets.Addon, enable bool) (bool, error) {
addonStatus, err := addon.IsEnabled()
func isAddonAlreadySet(addon *assets.Addon, enable bool, profile string) (bool, error) {
addonStatus, err := addon.IsEnabled(profile)
if err != nil {
return false, errors.Wrap(err, "addon is enabled")
return false, errors.Wrap(err, "is enabled")
}
if addonStatus && enable {
@ -227,6 +220,7 @@ func enableOrDisableAddonInternal(addon *assets.Addon, cmd command.Runner, data
// enableOrDisableStorageClasses enables or disables storage classes
func enableOrDisableStorageClasses(name, val, profile string) error {
glog.Infof("enableOrDisableStorageClasses %s=%v on %q", name, val, profile)
enable, err := strconv.ParseBool(val)
if err != nil {
return errors.Wrap(err, "Error parsing boolean")
@ -241,6 +235,17 @@ func enableOrDisableStorageClasses(name, val, profile string) error {
return errors.Wrapf(err, "Error getting storagev1 interface %v ", err)
}
api, err := machine.NewAPIClient()
if err != nil {
return errors.Wrap(err, "machine client")
}
defer api.Close()
if !cluster.IsHostRunning(api, profile) {
glog.Warningf("%q is not running, writing to %s=%v disk and skipping enablement", profile, name, val)
return enableOrDisableAddon(name, val, profile)
}
if enable {
// Only StorageClass for 'name' should be marked as default
err = storageclass.SetDefaultStorageClass(storagev1, class)
@ -269,7 +274,7 @@ func Start(profile string, additional []string) {
// Apply addons that are enabled by default
for name, a := range assets.Addons {
enabled, err := a.IsEnabled()
enabled, err := a.IsEnabled(profile)
if err != nil {
glog.Errorf("is-enabled failed for %q: %v", a.Name(), err)
continue

View File

@ -17,93 +17,115 @@ limitations under the License.
package addons
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
"testing"
"gotest.tools/assert"
"k8s.io/minikube/pkg/minikube/assets"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/localpath"
)
func createTestProfile(t *testing.T) string {
t.Helper()
td, err := ioutil.TempDir("", "profile")
if err != nil {
t.Fatalf("tempdir: %v", err)
}
err = os.Setenv(localpath.MinikubeHome, td)
if err != nil {
t.Errorf("error setting up test environment. could not set %s", localpath.MinikubeHome)
}
// Not necessary, but it is a handy random alphanumeric
name := filepath.Base(td)
if err := os.MkdirAll(config.ProfileFolderPath(name), 0777); err != nil {
t.Fatalf("error creating temporary directory")
}
if err := config.DefaultLoader.WriteConfigToFile(name, &config.MachineConfig{}); err != nil {
t.Fatalf("error creating temporary profile config: %v", err)
}
return name
}
func TestIsAddonAlreadySet(t *testing.T) {
testCases := []struct {
addonName string
}{
{
addonName: "ingress",
},
{
addonName: "registry",
},
profile := createTestProfile(t)
if err := Set("registry", "true", profile); err != nil {
t.Errorf("unable to set registry true: %v", err)
}
enabled, err := assets.Addons["registry"].IsEnabled(profile)
if err != nil {
t.Errorf("registry: %v", err)
}
if !enabled {
t.Errorf("expected registry to be enabled")
}
for _, test := range testCases {
addon := assets.Addons[test.addonName]
addonStatus, _ := addon.IsEnabled()
alreadySet, err := isAddonAlreadySet(addon, addonStatus)
if !alreadySet {
if addonStatus {
t.Errorf("Did not get expected status, \n\n expected %+v already enabled", test.addonName)
} else {
t.Errorf("Did not get expected status, \n\n expected %+v already disabled", test.addonName)
}
}
if err != nil {
t.Errorf("Got unexpected error: %+v", err)
}
enabled, err = assets.Addons["ingress"].IsEnabled(profile)
if err != nil {
t.Errorf("ingress: %v", err)
}
if enabled {
t.Errorf("expected ingress to not be enabled")
}
}
func TestDisableUnknownAddon(t *testing.T) {
tmpProfile := "temp-minikube-profile"
if err := Set("InvalidAddon", "false", tmpProfile); err == nil {
profile := createTestProfile(t)
if err := Set("InvalidAddon", "false", profile); err == nil {
t.Fatalf("Disable did not return error for unknown addon")
}
}
func TestEnableUnknownAddon(t *testing.T) {
tmpProfile := "temp-minikube-profile"
if err := Set("InvalidAddon", "true", tmpProfile); err == nil {
profile := createTestProfile(t)
if err := Set("InvalidAddon", "true", profile); err == nil {
t.Fatalf("Enable did not return error for unknown addon")
}
}
func TestEnableAndDisableAddon(t *testing.T) {
tests := []struct {
name string
enable bool
}{
{
name: "test enable",
enable: true,
}, {
name: "test disable",
enable: false,
},
profile := createTestProfile(t)
// enable
if err := Set("dashboard", "true", profile); err != nil {
t.Errorf("Disable returned unexpected error: " + err.Error())
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
tmpProfile := "temp-minikube-profile"
if err := os.MkdirAll(config.ProfileFolderPath(tmpProfile), 0777); err != nil {
t.Fatalf("error creating temporary directory")
}
defer os.RemoveAll(config.ProfileFolderPath(tmpProfile))
c, err := config.DefaultLoader.LoadConfigFromFile(profile)
if err != nil {
t.Errorf("unable to load profile: %v", err)
}
if c.Addons["dashboard"] != true {
t.Errorf("expected dashboard to be enabled")
}
if err := config.DefaultLoader.WriteConfigToFile(tmpProfile, &config.MachineConfig{}); err != nil {
t.Fatalf("error creating temporary profile config: %v", err)
}
if err := Set("dashboard", fmt.Sprintf("%t", test.enable), tmpProfile); err != nil {
t.Fatalf("Disable returned unexpected error: " + err.Error())
}
c, err := config.DefaultLoader.LoadConfigFromFile(tmpProfile)
if err != nil {
t.Fatalf("error loading config: %v", err)
}
assert.Equal(t, c.Addons["dashboard"], test.enable)
})
// disable
if err := Set("dashboard", "false", profile); err != nil {
t.Errorf("Disable returned unexpected error: " + err.Error())
}
c, err = config.DefaultLoader.LoadConfigFromFile(profile)
if err != nil {
t.Errorf("unable to load profile: %v", err)
}
if c.Addons["dashboard"] != false {
t.Errorf("expected dashboard to be enabled")
}
}
func TestStart(t *testing.T) {
profile := createTestProfile(t)
Start(profile, []string{"dashboard"})
enabled, err := assets.Addons["dashboard"].IsEnabled(profile)
if err != nil {
t.Errorf("dashboard: %v", err)
}
if !enabled {
t.Errorf("expected dashboard to be enabled")
}
}

View File

@ -25,7 +25,6 @@ import (
"github.com/golang/glog"
"github.com/pkg/errors"
"github.com/spf13/viper"
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/constants"
"k8s.io/minikube/pkg/minikube/localpath"
@ -55,16 +54,16 @@ func (a *Addon) Name() string {
return a.addonName
}
// IsEnabled checks if an Addon is enabled for the current profile
func (a *Addon) IsEnabled() (bool, error) {
c, err := config.Load(viper.GetString(config.MachineProfile))
// IsEnabled checks if an Addon is enabled for the given profile
func (a *Addon) IsEnabled(profile string) (bool, error) {
c, err := config.Load(profile)
if err != nil {
return false, errors.Wrap(err, "load")
}
// Is this addon explicitly listed in their configuration?
status, ok := c.Addons[a.Name()]
glog.Infof("IsEnabled %q = %v (listed in config=%v)", a.Name(), status, ok)
glog.V(1).Infof("IsEnabled %q = %v (listed in config=%v)", a.Name(), status, ok)
if ok {
return status, nil
}

View File

@ -627,17 +627,18 @@ func getIPForInterface(name string) (net.IP, error) {
// CheckIfHostExistsAndLoad checks if a host exists, and loads it if it does
func CheckIfHostExistsAndLoad(api libmachine.API, machineName string) (*host.Host, error) {
glog.Infof("Checking if %q exists ...", machineName)
exists, err := api.Exists(machineName)
if err != nil {
return nil, errors.Wrapf(err, "Error checking that machine exists: %s", machineName)
}
if !exists {
return nil, errors.Errorf("Machine does not exist for api.Exists(%s)", machineName)
return nil, errors.Errorf("machine %q does not exist", machineName)
}
host, err := api.Load(machineName)
if err != nil {
return nil, errors.Wrapf(err, "Error loading store for: %s", machineName)
return nil, errors.Wrapf(err, "loading machine %q", machineName)
}
return host, nil
}
@ -666,14 +667,15 @@ func CreateSSHShell(api libmachine.API, args []string) error {
return client.Shell(args...)
}
// IsMinikubeRunning checks that minikube has a status available and that
// the status is `Running`
func IsMinikubeRunning(api libmachine.API) bool {
s, err := GetHostStatus(api, viper.GetString(config.MachineProfile))
// IsHostRunning asserts that this profile's primary host is in state "Running"
func IsHostRunning(api libmachine.API, name string) bool {
s, err := GetHostStatus(api, name)
if err != nil {
glog.Warningf("host status for %q returned error: %v", name, err)
return false
}
if s != state.Running.String() {
glog.Warningf("%q host status: %s", name, s)
return false
}
return true