Skip to content

Commit

Permalink
kudoinit refactoring (#1491)
Browse files Browse the repository at this point in the history 
-- The Verify-Methods now don't return a verifier.Result anymore, but get a verifier.Result as a parameter and return a normal error to be more in sync with normal go error handling.
-- The prerequisites (namespace, service account, webhooks, etc.) are now simple kudoinit.Steps and not special extra subtypes.
  • Loading branch information
@ANeumann82
ANeumann82 committed May 5, 2020
1 parent e9cfc2b commit 6dd497d
Show file tree
Hide file tree
Showing 17 changed files with 451 additions and 495 deletions.
30 changes: 15 additions & 15 deletions pkg/controller/instance/instance_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,9 @@ func Test_makePipes(t *testing.T) {
plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{
{
Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{
{
Name: "step", Tasks: []string{}},
}},
{
Name: "step", Tasks: []string{}},
}},
}},
tasks: []v1beta1.Task{},
emeta: meta,
Expand All @@ -189,9 +189,9 @@ func Test_makePipes(t *testing.T) {
plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{
{
Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{
{
Name: "step", Tasks: []string{"task"}},
}},
{
Name: "step", Tasks: []string{"task"}},
}},
}},
tasks: []v1beta1.Task{
{
Expand All @@ -211,9 +211,9 @@ func Test_makePipes(t *testing.T) {
plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{
{
Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{
{
Name: "step", Tasks: []string{"task"}},
}},
{
Name: "step", Tasks: []string{"task"}},
}},
}},
tasks: []v1beta1.Task{
{
Expand Down Expand Up @@ -242,9 +242,9 @@ func Test_makePipes(t *testing.T) {
plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{
{
Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{
{Name: "stepOne", Tasks: []string{"task-one"}},
{Name: "stepTwo", Tasks: []string{"task-two"}},
}},
{Name: "stepOne", Tasks: []string{"task-one"}},
{Name: "stepTwo", Tasks: []string{"task-two"}},
}},
}},
tasks: []v1beta1.Task{
{
Expand Down Expand Up @@ -292,9 +292,9 @@ func Test_makePipes(t *testing.T) {
plan: &v1beta1.Plan{Strategy: "serial", Phases: []v1beta1.Phase{
{
Name: "phase", Strategy: "serial", Steps: []v1beta1.Step{
{
Name: "step", Tasks: []string{"task"}},
}},
{
Name: "step", Tasks: []string{"task"}},
}},
}},
tasks: []v1beta1.Task{
{
Expand Down
23 changes: 23 additions & 0 deletions pkg/kudoctl/cmd/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"io"
"strings"

"github.com/kudobuilder/kudo/pkg/kudoctl/verifier"

"github.com/spf13/afero"
"github.com/spf13/cobra"
flag "github.com/spf13/pflag"
Expand Down Expand Up @@ -179,6 +181,13 @@ func (initCmd *initCmd) run() error {
}
initCmd.client = client
}
ok, err := initCmd.preInstallVerify(opts)
if err != nil {
return err
}
if !ok {
return fmt.Errorf("failed to verify installation requirements")
}

if err := setup.Install(initCmd.client, opts, initCmd.crdOnly); err != nil {
return clog.Errorf("error installing: %s", err)
Expand All @@ -196,6 +205,20 @@ func (initCmd *initCmd) run() error {
return nil
}

// preInstallVerify runs the pre-installation verification and returns true if the installation can continue
func (initCmd *initCmd) preInstallVerify(opts kudoinit.Options) (bool, error) {
result := verifier.NewResult()
if err := setup.PreInstallVerify(initCmd.client, opts, initCmd.crdOnly, &result); err != nil {
return false, err
}
result.PrintWarnings(initCmd.out)
if !result.IsValid() {
result.PrintErrors(initCmd.out)
return false, nil
}
return true, nil
}

func webhooksArray(webhooksAsStr string) []string {
if webhooksAsStr == "" {
return []string{}
Expand Down
213 changes: 108 additions & 105 deletions pkg/kudoctl/cmd/init_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ func assertManifestFileMatch(t *testing.T, fileName string, expectedObject runti
assert.Equal(t, string(expectedContent), string(of), fmt.Sprintf("embedded file %s does not match the source, run 'make generate'", fileName))
}

func assertStringContains(t *testing.T, expected string, actual string) {
assert.True(t, strings.Contains(actual, expected), "Expected to find '%s' in '%s'", expected, actual)
}

func runtimeObjectAsBytes(o runtime.Object) ([]byte, error) {
bytes, err := yaml.Marshal(o)
if err != nil {
Expand Down Expand Up @@ -145,7 +149,8 @@ func TestIntegInitWithNameSpace(t *testing.T) {
// On first attempt, the namespace does not exist, so the error is expected.
err = cmd.run()
require.Error(t, err)
assert.Equal(t, "error installing: Namespace integration-test does not exist - KUDO expects that any namespace except the default kudo-system is created beforehand\n", err.Error())
assert.Equal(t, "failed to verify installation requirements", err.Error())
assertStringContains(t, "Namespace integration-test does not exist - KUDO expects that any namespace except the default kudo-system is created beforehand", buf.String())

// Then we manually create the namespace.
ns := testutils.NewResource("v1", "Namespace", namespace, "")
Expand Down Expand Up @@ -187,111 +192,105 @@ func TestIntegInitWithNameSpace(t *testing.T) {
4. Run Init command with a serviceAccount that does have cluster-admin role, but not in the expected namespace.
5. Run Init command with a serviceAccount that is present in the cluster and also has cluster-admin role.
*/

func TestIntegInitWithServiceAccount(t *testing.T) {
namespace := "sa-integration-test"
serviceAccount := "sa-integration"
// Kubernetes client caches the types, se we need to re-initialize it.
testClient, err := testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient := getKubeClient(t)

instance := testutils.NewResource("kudo.dev/v1beta1", "Instance", "zk", "ns")
// Verify that we cannot create the instance, because the test environment is empty.
assert.IsType(t, &meta.NoKindMatchError{}, testClient.Create(context.TODO(), instance))

// Install all of the CRDs.
crds := crd.NewInitializer().Resources()
defer deleteInitObjects(testClient)

var buf bytes.Buffer
cmd := &initCmd{
out: &buf,
fs: afero.NewMemMapFs(),
client: kclient,
ns: namespace,
serviceAccount: "test-account",
func TestInitWithServiceAccount(t *testing.T) {
tests := []struct {
name string
serviceAccount string
roleBindingRole string
roleBindingNs string
errMessageContains string
}{
{name: "service account not present", serviceAccount: "", errMessageContains: "Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test-0"},
{name: "service account has no rb", serviceAccount: "test-account", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-1 and to have cluster-admin role"},
{name: "rb has no cluster-admin role", serviceAccount: "test-account", roleBindingRole: "not-admin", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-2 and to have cluster-admin role"},
{name: "rb has different ns", serviceAccount: "test-account", roleBindingRole: "not-admin", roleBindingNs: "otherns", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-3 and to have cluster-admin role"},
{name: "rb has admin in different ns", serviceAccount: "test-account", roleBindingRole: "cluster-admin", roleBindingNs: "otherns", errMessageContains: "Service Account test-account does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test-4 and to have cluster-admin role"},
{name: "rb has cluster-admin role", serviceAccount: "test-account", roleBindingRole: "cluster-admin", errMessageContains: ""},
}

// Manually create the namespace and the serviceAccount to be used later
ns := testutils.NewResource("v1", "Namespace", namespace, "")
assert.NoError(t, testClient.Create(context.TODO(), ns))
defer testClient.Delete(context.TODO(), ns)
sa := testutils.NewResource("v1", "ServiceAccount", serviceAccount, namespace)
assert.NoError(t, testClient.Create(context.TODO(), sa))
defer testClient.Delete(context.TODO(), sa)

// Test Case 1, the serviceAccount does not exist, expect serviceAccount not exists error
err = cmd.run()
require.Error(t, err)
assert.Equal(t, "error installing: Service Account test-account does not exists - KUDO expects the serviceAccount to be present in the namespace sa-integration-test\n", err.Error())

// Create the serviceAccount, in the default namespace.
ns2 := testutils.NewResource("v1", "Namespace", "test-ns", "")
assert.NoError(t, testClient.Create(context.TODO(), ns2))
defer testClient.Delete(context.TODO(), ns2)
sa2 := testutils.NewResource("v1", "ServiceAccount", "sa-nonadmin", "test-ns")
assert.NoError(t, testClient.Create(context.TODO(), sa2))
defer testClient.Delete(context.TODO(), sa2)

// Test Case 2, the serviceAccount exists, but does not part of clusterrolebindings
cmd.serviceAccount = "sa-nonadmin"
cmd.ns = "test-ns"
err = cmd.run()
require.Error(t, err)
assert.Equal(t, "error installing: Service Account sa-nonadmin does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace test-ns and to have cluster-admin role\n", err.Error())

// Test case 3: Run Init command with a serviceAccount that does not have cluster-admin role.
cmd.serviceAccount = serviceAccount
cmd.ns = namespace
crb := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-test1", "test-ns", serviceAccount, "cluster-temp")
assert.NoError(t, testClient.Create(context.TODO(), crb))
defer testClient.Delete(context.TODO(), crb)

err = cmd.run()
require.Error(t, err)
assert.Equal(t, "error installing: Service Account sa-integration does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test and to have cluster-admin role\n", err.Error())

// Test case 4: Run Init command with a serviceAccount that does not have cluster-admin role.
crb2 := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-test2", namespace, serviceAccount, "cluster-temp")
assert.NoError(t, testClient.Create(context.TODO(), crb2))
defer testClient.Delete(context.TODO(), crb2)

err = cmd.run()
require.Error(t, err)
assert.Equal(t, "error installing: Service Account sa-integration does not have cluster-admin role - KUDO expects the serviceAccount passed to be in the namespace sa-integration-test and to have cluster-admin role\n", err.Error())

// Test case 5: Run Init command with a serviceAccount that is present in the cluster and also has cluster-admin role.
crb3 := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-clusterrole-binding", namespace, serviceAccount, "cluster-admin")
assert.NoError(t, testClient.Create(context.TODO(), crb3))
defer testClient.Delete(context.TODO(), crb3)

err = cmd.run()
assert.NoError(t, err)

// WaitForCRDs to be created... the init cmd did NOT wait
assert.Nil(t, testutils.WaitForCRDs(testenv.DiscoveryClient, crds))

// Kubernetes client caches the types, so we need to re-initialize it.
testClient, err = testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient = getKubeClient(t)

// make sure that the controller lives in the correct namespace
statefulsets, err := kclient.KubeClient.AppsV1().StatefulSets(namespace).List(metav1.ListOptions{})
assert.Nil(t, err)

kudoControllerFound := false
for _, ss := range statefulsets.Items {
if ss.Name == kudoinit.DefaultManagerName {
kudoControllerFound = true
}
namespaceBase := "sa-integration-test"

for idx, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
namespace := fmt.Sprintf("%s-%d", namespaceBase, idx)

// Kubernetes client caches the types, se we need to re-initialize it.
testClient, err := testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient := getKubeClient(t)

instance := testutils.NewResource("kudo.dev/v1beta1", "Instance", "zk", "ns")
// Verify that we cannot create the instance, because the test environment is empty.
assert.IsType(t, &meta.NoKindMatchError{}, testClient.Create(context.TODO(), instance))

// Install all of the CRDs.
crds := crd.NewInitializer().Resources()
defer deleteInitObjects(testClient)

var buf bytes.Buffer
cmd := &initCmd{
out: &buf,
fs: afero.NewMemMapFs(),
client: kclient,
ns: namespace,
serviceAccount: "test-account",
}

ns := testutils.NewResource("v1", "Namespace", namespace, "")
assert.NoError(t, testClient.Create(context.TODO(), ns))
defer testClient.Delete(context.TODO(), ns)

if tt.serviceAccount != "" {
sa2 := testutils.NewResource("v1", "ServiceAccount", tt.serviceAccount, namespace)
assert.NoError(t, testClient.Create(context.TODO(), sa2))
defer testClient.Delete(context.TODO(), sa2)
}

if tt.roleBindingRole != "" {
rbNamespace := tt.roleBindingNs
if rbNamespace == "" {
rbNamespace = namespace
}
crb := testutils.NewClusterRoleBinding("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "kudo-clusterrole-binding", rbNamespace, tt.serviceAccount, tt.roleBindingRole)
assert.NoError(t, testClient.Create(context.TODO(), crb))
defer testClient.Delete(context.TODO(), crb)
}

err = cmd.run()

if tt.errMessageContains != "" {
require.Error(t, err)
assert.Equal(t, "failed to verify installation requirements", err.Error())
assertStringContains(t, tt.errMessageContains, buf.String())
} else {
assert.NoError(t, err)

// WaitForCRDs to be created... the init cmd did NOT wait
assert.Nil(t, testutils.WaitForCRDs(testenv.DiscoveryClient, crds))

// Kubernetes client caches the types, so we need to re-initialize it.
testClient, err = testutils.NewRetryClient(testenv.Config, client.Options{
Scheme: testutils.Scheme(),
})
assert.Nil(t, err)
kclient = getKubeClient(t)

// make sure that the controller lives in the correct namespace
statefulsets, err := kclient.KubeClient.AppsV1().StatefulSets(namespace).List(metav1.ListOptions{})
assert.Nil(t, err)

kudoControllerFound := false
for _, ss := range statefulsets.Items {
if ss.Name == kudoinit.DefaultManagerName {
kudoControllerFound = true
}
}
assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace))
}
})
}
assert.True(t, kudoControllerFound, fmt.Sprintf("No kudo-controller-manager statefulset found in namespace %s", namespace))
}

func TestNoErrorOnReInit(t *testing.T) {
Expand Down Expand Up @@ -340,10 +339,14 @@ func TestNoErrorOnReInit(t *testing.T) {
}

func deleteInitObjects(client *testutils.RetryClient) {
opts := kudoinit.NewOptions("", "", "", []string{}, false)

crds := crd.NewInitializer()
prereqs := prereq.NewInitializer(kudoinit.NewOptions("", "", "", []string{}, false))

deleteCRDs(crds.Resources(), client)
deletePrereq(prereqs.Resources(), client)
deletePrereq(prereq.NewNamespaceInitializer(opts).Resources(), client)
deletePrereq(prereq.NewServiceAccountInitializer(opts).Resources(), client)
deletePrereq(prereq.NewWebHookInitializer(opts).Resources(), client)
}

func deleteCRDs(crds []runtime.Object, client *testutils.RetryClient) {
Expand Down
Loading

0 comments on commit 6dd497d

Please sign in to comment.