Unit test install strategy ALM-227 #132
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| opClient "github.com/coreos-inc/operator-client/pkg/client" | ||
| log "github.com/sirupsen/logrus" | ||
| "github.com/pkg/errors" | ||
| "k8s.io/api/core/v1" | ||
| v1beta1extensions "k8s.io/api/extensions/v1beta1" | ||
| v1beta1rbac "k8s.io/api/rbac/v1beta1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| type InstallStrategyDeploymentInterface interface { | ||
| CreateRole(role *v1beta1rbac.Role) (*v1beta1rbac.Role, error) | ||
| CreateRoleBinding(roleBinding *v1beta1rbac.RoleBinding) (*v1beta1rbac.RoleBinding, error) | ||
| GetOrCreateServiceAccount(serviceAccount *v1.ServiceAccount) (*v1.ServiceAccount, error) | ||
|
There was a problem hiding this comment. The expected behavior of this function isn't very clear from the name. I think simply making it There was a problem hiding this comment. It actually does return the ServiceAccount it finds if it's already there - it's not a no-op if it already exists. There was a problem hiding this comment. It didn't look like anything other than the name, which is set in alm/install/deployment and by necessity the same. Should this really be an upsert, i.e. |
||
| CreateDeployment(deployment *v1beta1extensions.Deployment) (*v1beta1extensions.Deployment, error) | ||
| CheckServiceAccount(serviceAccountName string) (bool, error) | ||
|
There was a problem hiding this comment. Check what? It might be simpler to just have this be the simple |
||
| CheckOwnedDeployments(owner metav1.ObjectMeta, deploymentSpecs []v1beta1extensions.DeploymentSpec) (bool, error) | ||
|
There was a problem hiding this comment. Same here. This function weirdly mixes client and consumer concerns. Doing |
||
| } | ||
|
|
||
| type InstallStrategyDeploymentClient struct { | ||
| opClient opClient.Interface | ||
| Namespace string | ||
|
There was a problem hiding this comment. I'm unclear on how this namespace is used. Are all actions scoped to this namespace? As a consumer of this client, it's very opaque. Either consistently use There was a problem hiding this comment. Yeah, all actions are restricted to the namespace. I can make that clear with the name |
||
| } | ||
|
|
||
| var _ InstallStrategyDeploymentInterface = &InstallStrategyDeploymentClient{} | ||
|
|
||
| func NewInstallStrategyDeploymentClient(opClient opClient.Interface, namespace string) InstallStrategyDeploymentInterface { | ||
| return &InstallStrategyDeploymentClient{ | ||
| opClient: opClient, | ||
| Namespace: namespace, | ||
| } | ||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) CreateRole(role *v1beta1rbac.Role) (*v1beta1rbac.Role, error) { | ||
| return c.opClient.KubernetesInterface().RbacV1beta1().Roles(c.Namespace).Create(role) | ||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) CreateRoleBinding(roleBinding *v1beta1rbac.RoleBinding) (*v1beta1rbac.RoleBinding, error) { | ||
| return c.opClient.KubernetesInterface().RbacV1beta1().RoleBindings(c.Namespace).Create(roleBinding) | ||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) GetOrCreateServiceAccount(serviceAccount *v1.ServiceAccount) (*v1.ServiceAccount, error) { | ||
| foundAccount, err := c.opClient.KubernetesInterface().CoreV1().ServiceAccounts(c.Namespace).Get(serviceAccount.Name, metav1.GetOptions{}) | ||
| if err != nil && !apierrors.IsNotFound(err) { | ||
| return nil, errors.Wrap(err, "checking for existing serviceacccount failed") | ||
| } | ||
| if foundAccount != nil { | ||
| return foundAccount, nil | ||
| } | ||
|
|
||
| createdAccount, err := c.opClient.KubernetesInterface().CoreV1().ServiceAccounts(c.Namespace).Create(serviceAccount) | ||
| if err != nil && !apierrors.IsAlreadyExists(err) { | ||
| return createdAccount, errors.Wrap(err, "creating serviceacccount failed") | ||
| } | ||
| return createdAccount, nil | ||
|
There was a problem hiding this comment. Do you need to re-fetch the serviceaccount if you get a There was a problem hiding this comment. Nope, we only need the name to create the rolebinding There was a problem hiding this comment. But won't There was a problem hiding this comment. It will be nil, but that's the correct value for it for this return. I've changed it explicitly to nil so that it's clear. |
||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) CreateDeployment(deployment *v1beta1extensions.Deployment) (*v1beta1extensions.Deployment, error) { | ||
| return c.opClient.CreateDeployment(deployment) | ||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) CheckServiceAccount(serviceAccountName string) (bool, error) { | ||
| if _, err := c.opClient.KubernetesInterface().CoreV1().ServiceAccounts(c.Namespace).Get(serviceAccountName, metav1.GetOptions{}); err != nil { | ||
| if apierrors.IsNotFound(err) { | ||
| return false, nil | ||
| } | ||
| return false, fmt.Errorf("serviceaccount %s not found: %s", serviceAccountName, err.Error()) | ||
| } | ||
| return true, nil | ||
| } | ||
|
|
||
| func (c *InstallStrategyDeploymentClient) CheckOwnedDeployments(owner metav1.ObjectMeta, deploymentSpecs []v1beta1extensions.DeploymentSpec) (bool, error) { | ||
| existingDeployments, err := c.opClient.ListDeploymentsWithLabels(c.Namespace, map[string]string{ | ||
| "alm-owner-name": owner.Name, | ||
| "alm-owner-namespace": owner.Namespace, | ||
|
There was a problem hiding this comment. Personal pref, I would prefer to have There was a problem hiding this comment. We might change this query to user ownerreferences in the future, so I think it makes sense to keep the whole owner There was a problem hiding this comment. I think I have some fear still from experiences where parts of the codebase break unexpectedly when you change something about the object because it relied on some obscure field on the object you didn't even know about. I'll trust your judgement on where we need flexibility more than guaranteeing future stability. 👍 |
||
| }) | ||
| if err != nil { | ||
| return false, fmt.Errorf("couldn't query for existing deployments: %s", err) | ||
| } | ||
| if len(existingDeployments.Items) != len(deploymentSpecs) { | ||
| log.Debugf("wrong number of deployments found. want %d, got %d", len(deploymentSpecs), len(existingDeployments.Items)) | ||
| return false, nil | ||
| } | ||
| return true, nil | ||
| } | ||
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,59 +1,51 @@ | ||
| // Automatically generated by MockGen. DO NOT EDIT! | ||
| // Source: client/clusterserviceversion_client.go | ||
|
|
||
| package client | ||
|
|
||
| import ( | ||
| v1alpha1 "github.com/coreos-inc/alm/apis/clusterserviceversion/v1alpha1" | ||
| gomock "github.com/golang/mock/gomock" | ||
| ) | ||
|
|
||
| // Mock of ClusterServiceVersionInterface interface | ||
| type MockClusterServiceVersionInterface struct { | ||
| ctrl *gomock.Controller | ||
| recorder *_MockClusterServiceVersionInterfaceRecorder | ||
| } | ||
|
|
||
| // Recorder for MockClusterServiceVersionInterface (not exported) | ||
| type _MockClusterServiceVersionInterfaceRecorder struct { | ||
| mock *MockClusterServiceVersionInterface | ||
| } | ||
|
|
||
| func NewMockClusterServiceVersionInterface(ctrl *gomock.Controller) *MockClusterServiceVersionInterface { | ||
| mock := &MockClusterServiceVersionInterface{ctrl: ctrl} | ||
| mock.recorder = &_MockClusterServiceVersionInterfaceRecorder{mock} | ||
| return mock | ||
| } | ||
|
|
||
| func (_m *MockClusterServiceVersionInterface) EXPECT() *_MockClusterServiceVersionInterfaceRecorder { | ||
| return _m.recorder | ||
| } | ||
|
|
||
| func (_m *MockClusterServiceVersionInterface) UpdateCSV(csv *v1alpha1.ClusterServiceVersion) (*v1alpha1.ClusterServiceVersion, error) { | ||
| ret := _m.ctrl.Call(_m, "UpdateCSV", csv) | ||
| ret0, _ := ret[0].(*v1alpha1.ClusterServiceVersion) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockClusterServiceVersionInterfaceRecorder) UpdateCSV(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "UpdateCSV", arg0) | ||
| } | ||
|
|
||
| func (_m *MockClusterServiceVersionInterface) CreateCSV(csv *v1alpha1.ClusterServiceVersion) error { | ||
| ret := _m.ctrl.Call(_m, "CreateCSV", csv) | ||
| ret0, _ := ret[0].(error) | ||
| return ret0 | ||
| } | ||
|
|
||
| func (_mr *_MockClusterServiceVersionInterfaceRecorder) CreateCSV(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CreateCSV", arg0) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Automatically generated by MockGen. DO NOT EDIT! | ||
| // Source: client/deployment_install_client.go | ||
|
|
||
| package client | ||
|
|
||
| import ( | ||
| gomock "github.com/golang/mock/gomock" | ||
| v1 "k8s.io/api/core/v1" | ||
| v1beta10 "k8s.io/api/extensions/v1beta1" | ||
| v1beta1 "k8s.io/api/rbac/v1beta1" | ||
| v10 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| ) | ||
|
|
||
| // Mock of InstallStrategyDeploymentInterface interface | ||
| type MockInstallStrategyDeploymentInterface struct { | ||
| ctrl *gomock.Controller | ||
| recorder *_MockInstallStrategyDeploymentInterfaceRecorder | ||
| } | ||
|
|
||
| // Recorder for MockInstallStrategyDeploymentInterface (not exported) | ||
| type _MockInstallStrategyDeploymentInterfaceRecorder struct { | ||
| mock *MockInstallStrategyDeploymentInterface | ||
| } | ||
|
|
||
| func NewMockInstallStrategyDeploymentInterface(ctrl *gomock.Controller) *MockInstallStrategyDeploymentInterface { | ||
| mock := &MockInstallStrategyDeploymentInterface{ctrl: ctrl} | ||
| mock.recorder = &_MockInstallStrategyDeploymentInterfaceRecorder{mock} | ||
| return mock | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) EXPECT() *_MockInstallStrategyDeploymentInterfaceRecorder { | ||
| return _m.recorder | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) CreateRole(role *v1beta1.Role) (*v1beta1.Role, error) { | ||
| ret := _m.ctrl.Call(_m, "CreateRole", role) | ||
| ret0, _ := ret[0].(*v1beta1.Role) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) CreateRole(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CreateRole", arg0) | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) CreateRoleBinding(roleBinding *v1beta1.RoleBinding) (*v1beta1.RoleBinding, error) { | ||
| ret := _m.ctrl.Call(_m, "CreateRoleBinding", roleBinding) | ||
| ret0, _ := ret[0].(*v1beta1.RoleBinding) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) CreateRoleBinding(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CreateRoleBinding", arg0) | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) GetOrCreateServiceAccount(serviceAccount *v1.ServiceAccount) (*v1.ServiceAccount, error) { | ||
| ret := _m.ctrl.Call(_m, "GetOrCreateServiceAccount", serviceAccount) | ||
| ret0, _ := ret[0].(*v1.ServiceAccount) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) GetOrCreateServiceAccount(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "GetOrCreateServiceAccount", arg0) | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) CreateDeployment(deployment *v1beta10.Deployment) (*v1beta10.Deployment, error) { | ||
| ret := _m.ctrl.Call(_m, "CreateDeployment", deployment) | ||
| ret0, _ := ret[0].(*v1beta10.Deployment) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) CreateDeployment(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CreateDeployment", arg0) | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) CheckServiceAccount(serviceAccountName string) (bool, error) { | ||
| ret := _m.ctrl.Call(_m, "CheckServiceAccount", serviceAccountName) | ||
| ret0, _ := ret[0].(bool) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) CheckServiceAccount(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CheckServiceAccount", arg0) | ||
| } | ||
|
|
||
| func (_m *MockInstallStrategyDeploymentInterface) CheckOwnedDeployments(owner v10.ObjectMeta, deploymentSpecs []v1beta10.DeploymentSpec) (bool, error) { | ||
| ret := _m.ctrl.Call(_m, "CheckOwnedDeployments", owner, deploymentSpecs) | ||
| ret0, _ := ret[0].(bool) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallStrategyDeploymentInterfaceRecorder) CheckOwnedDeployments(arg0, arg1 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "CheckOwnedDeployments", arg0, arg1) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,47 +1,41 @@ | ||
| // Automatically generated by MockGen. DO NOT EDIT! | ||
| // Source: client/installplan_client.go | ||
|
|
||
| package client | ||
|
|
||
| import ( | ||
| v1alpha1 "github.com/coreos-inc/alm/apis/installplan/v1alpha1" | ||
| gomock "github.com/golang/mock/gomock" | ||
| ) | ||
|
|
||
| // Mock of InstallPlanInterface interface | ||
| type MockInstallPlanInterface struct { | ||
| ctrl *gomock.Controller | ||
| recorder *_MockInstallPlanInterfaceRecorder | ||
| } | ||
|
|
||
| // Recorder for MockInstallPlanInterface (not exported) | ||
| type _MockInstallPlanInterfaceRecorder struct { | ||
| mock *MockInstallPlanInterface | ||
| } | ||
|
|
||
| func NewMockInstallPlanInterface(ctrl *gomock.Controller) *MockInstallPlanInterface { | ||
| mock := &MockInstallPlanInterface{ctrl: ctrl} | ||
| mock.recorder = &_MockInstallPlanInterfaceRecorder{mock} | ||
| return mock | ||
| } | ||
|
|
||
| func (_m *MockInstallPlanInterface) EXPECT() *_MockInstallPlanInterfaceRecorder { | ||
| return _m.recorder | ||
| } | ||
|
|
||
| func (_m *MockInstallPlanInterface) UpdateInstallPlan(_param0 *v1alpha1.InstallPlan) (*v1alpha1.InstallPlan, error) { | ||
| ret := _m.ctrl.Call(_m, "UpdateInstallPlan", _param0) | ||
| ret0, _ := ret[0].(*v1alpha1.InstallPlan) | ||
| ret1, _ := ret[1].(error) | ||
| return ret0, ret1 | ||
| } | ||
|
|
||
| func (_mr *_MockInstallPlanInterfaceRecorder) UpdateInstallPlan(arg0 interface{}) *gomock.Call { | ||
| return _mr.mock.ctrl.RecordCall(_mr.mock, "UpdateInstallPlan", arg0) | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InstallStraegyInvalidreads a little weird to me. Can we reverse the order on it, i.e.InvalidInstallStrategy?