Skip to content

Commit

Permalink
List all matchable attributes (flyteorg#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
katrogan authored Feb 13, 2020
1 parent 6a64f00 commit 15d1fa0
Show file tree
Hide file tree
Showing 17 changed files with 283 additions and 16 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ require (
github.com/json-iterator/go v1.1.9 // indirect
github.com/kelseyhightower/envconfig v1.4.0 // indirect
github.com/lib/pq v1.3.0
github.com/lyft/flyteidl v0.17.0
github.com/lyft/flyteidl v0.17.3
github.com/lyft/flytepropeller v0.1.30
github.com/lyft/flytestdlib v0.3.0
github.com/magiconair/properties v1.8.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,8 @@ github.com/lyft/api v0.0.0-20191031200350-b49a72c274e0 h1:NGL46+1RYcCXb3sShp0nQq
github.com/lyft/api v0.0.0-20191031200350-b49a72c274e0/go.mod h1:/L5qH+AD540e7Cetbui1tuJeXdmNhO8jM6VkXeDdDhQ=
github.com/lyft/apimachinery v0.0.0-20191031200210-047e3ea32d7f h1:PGuAMDzAen0AulUfaEhNQMYmUpa41pAVo3zHI+GJsCM=
github.com/lyft/apimachinery v0.0.0-20191031200210-047e3ea32d7f/go.mod h1:llRdnznGEAqC3DcNm6yEj472xaFVfLM7hnYofMb12tQ=
github.com/lyft/flyteidl v0.17.0 h1:Vsg38PGQAe+A1/9y6buFA5HoZO5vjAT7XNKrWokz084=
github.com/lyft/flyteidl v0.17.0/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/F1Cu20=
github.com/lyft/flyteidl v0.17.3 h1:ihMrx+ipLkKtJ4h9s32JcUgdPW+VmcsFq3k0yoJc8FY=
github.com/lyft/flyteidl v0.17.3/go.mod h1:/zQXxuHO11u/saxTTZc8oYExIGEShXB+xCB1/F1Cu20=
github.com/lyft/flytepropeller v0.1.30 h1:g55bD3aMMba4WDiBE7SLFEElutPdkEtoFQkgN59OX+M=
github.com/lyft/flytepropeller v0.1.30/go.mod h1:SgMi8FEw9K8BZHggUXIQ5Maw8LF9ymgtNTDjNahmXOc=
github.com/lyft/flytestdlib v0.3.0 h1:nIkX4MlyYdcLLzaF35RI2P5BhARt+qMgHoFto8eVNzU=
Expand Down
22 changes: 22 additions & 0 deletions pkg/manager/impl/resources/resource_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,28 @@ func (m *ResourceManager) DeleteProjectDomainAttributes(ctx context.Context,
return &admin.ProjectDomainAttributesDeleteResponse{}, nil
}

func (m *ResourceManager) ListAll(ctx context.Context, request admin.ListMatchableAttributesRequest) (
*admin.ListMatchableAttributesResponse, error) {
if err := validation.ValidateListAllMatchableAttributesRequest(request); err != nil {
return nil, err
}
resources, err := m.db.ResourceRepo().ListAll(ctx, request.ResourceType.String())
if err != nil {
return nil, err
}
if resources == nil {
// That's fine - there don't necessarily need to exist overrides in the database
return &admin.ListMatchableAttributesResponse{}, nil
}
configurations, err := transformers.FromResourceModelsToMatchableAttributes(resources)
if err != nil {
return nil, err
}
return &admin.ListMatchableAttributesResponse{
Configurations: configurations,
}, nil
}

func NewResourceManager(db repositories.RepositoryInterface) interfaces.ResourceInterface {
return &ResourceManager{
db: db,
Expand Down
75 changes: 67 additions & 8 deletions pkg/manager/impl/resources/resource_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"context"
"testing"

interfaces2 "github.com/lyft/flyteadmin/pkg/manager/interfaces"
"github.com/lyft/flyteadmin/pkg/repositories/interfaces"
"github.com/lyft/flyteadmin/pkg/manager/interfaces"
repoInterfaces "github.com/lyft/flyteadmin/pkg/repositories/interfaces"

"github.com/golang/protobuf/proto"
"github.com/lyft/flyteadmin/pkg/manager/impl/testutils"
Expand Down Expand Up @@ -56,7 +56,7 @@ func TestGetWorkflowAttributes(t *testing.T) {
}
db := mocks.NewMockRepository()
db.ResourceRepo().(*mocks.MockResourceRepo).GetFunction = func(
ctx context.Context, ID interfaces.ResourceID) (models.Resource, error) {
ctx context.Context, ID repoInterfaces.ResourceID) (models.Resource, error) {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, workflow, ID.Workflow)
Expand Down Expand Up @@ -92,7 +92,7 @@ func TestDeleteWorkflowAttributes(t *testing.T) {
}
db := mocks.NewMockRepository()
db.ResourceRepo().(*mocks.MockResourceRepo).DeleteFunction = func(
ctx context.Context, ID interfaces.ResourceID) error {
ctx context.Context, ID repoInterfaces.ResourceID) error {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, workflow, ID.Workflow)
Expand Down Expand Up @@ -139,7 +139,7 @@ func TestGetProjectDomainAttributes(t *testing.T) {
}
db := mocks.NewMockRepository()
db.ResourceRepo().(*mocks.MockResourceRepo).GetFunction = func(
ctx context.Context, ID interfaces.ResourceID) (models.Resource, error) {
ctx context.Context, ID repoInterfaces.ResourceID) (models.Resource, error) {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, "", ID.Workflow)
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestDeleteProjectDomainAttributes(t *testing.T) {
}
db := mocks.NewMockRepository()
db.ResourceRepo().(*mocks.MockResourceRepo).DeleteFunction = func(
ctx context.Context, ID interfaces.ResourceID) error {
ctx context.Context, ID repoInterfaces.ResourceID) error {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, admin.MatchableResource_EXECUTION_QUEUE.String(), ID.ResourceType)
Expand All @@ -184,7 +184,7 @@ func TestDeleteProjectDomainAttributes(t *testing.T) {
}

func TestGetResource(t *testing.T) {
request := interfaces2.ResourceRequest{
request := interfaces.ResourceRequest{
Project: project,
Domain: domain,
Workflow: workflow,
Expand All @@ -193,7 +193,7 @@ func TestGetResource(t *testing.T) {
}
db := mocks.NewMockRepository()
db.ResourceRepo().(*mocks.MockResourceRepo).GetFunction = func(
ctx context.Context, ID interfaces.ResourceID) (models.Resource, error) {
ctx context.Context, ID repoInterfaces.ResourceID) (models.Resource, error) {
assert.Equal(t, project, ID.Project)
assert.Equal(t, domain, ID.Domain)
assert.Equal(t, workflow, ID.Workflow)
Expand All @@ -219,3 +219,62 @@ func TestGetResource(t *testing.T) {
assert.Equal(t, request.ResourceType.String(), response.ResourceType)
assert.True(t, proto.Equal(response.Attributes, testutils.ExecutionQueueAttributes))
}

func TestListAllResources(t *testing.T) {
db := mocks.NewMockRepository()
projectAttributes := admin.MatchingAttributes{
Target: &admin.MatchingAttributes_ClusterResourceAttributes{
ClusterResourceAttributes: &admin.ClusterResourceAttributes{
Attributes: map[string]string{
"foo": "foofoo",
},
},
},
}
marshaledProjectAttrs, _ := proto.Marshal(&projectAttributes)
workflowAttributes := admin.MatchingAttributes{
Target: &admin.MatchingAttributes_ClusterResourceAttributes{
ClusterResourceAttributes: &admin.ClusterResourceAttributes{
Attributes: map[string]string{
"bar": "barbar",
},
},
},
}
marshaledWorkflowAttrs, _ := proto.Marshal(&workflowAttributes)
db.ResourceRepo().(*mocks.MockResourceRepo).ListAllFunction = func(ctx context.Context, resourceType string) (
[]models.Resource, error) {
assert.Equal(t, admin.MatchableResource_CLUSTER_RESOURCE.String(), resourceType)
return []models.Resource{
{
Project: "projectA",
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE.String(),
Attributes: marshaledProjectAttrs,
},
{
Project: "projectB",
Domain: "development",
Workflow: "workflow",
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE.String(),
Attributes: marshaledWorkflowAttrs,
},
}, nil
}
manager := NewResourceManager(db)
response, err := manager.ListAll(context.Background(), admin.ListMatchableAttributesRequest{
ResourceType: admin.MatchableResource_CLUSTER_RESOURCE,
})
assert.Nil(t, err)
assert.NotNil(t, response.Configurations)
assert.Len(t, response.Configurations, 2)
assert.True(t, proto.Equal(&admin.MatchableAttributesConfiguration{
Project: "projectA",
Attributes: &projectAttributes,
}, response.Configurations[0]))
assert.True(t, proto.Equal(&admin.MatchableAttributesConfiguration{
Project: "projectB",
Domain: "development",
Workflow: "workflow",
Attributes: &workflowAttributes,
}, response.Configurations[1]))
}
2 changes: 1 addition & 1 deletion pkg/manager/impl/shared/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const (
Event = "event"
ParentTaskExecutionID = "parent_task_execution_id"
UserInputs = "user_inputs"
ProjectDomain = "project_domain"
Attributes = "attributes"
MatchingAttributes = "matching_attributes"
Resourcetype = "resource_type"
)
7 changes: 7 additions & 0 deletions pkg/manager/impl/validation/attributes_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,10 @@ func ValidateWorkflowAttributesDeleteRequest(request admin.WorkflowAttributesDel

return nil
}

func ValidateListAllMatchableAttributesRequest(request admin.ListMatchableAttributesRequest) error {
if _, ok := admin.MatchableResource_name[int32(request.ResourceType)]; !ok {
return shared.GetInvalidArgumentError(shared.ResourceType)
}
return nil
}
12 changes: 12 additions & 0 deletions pkg/manager/impl/validation/attributes_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,15 @@ func TestValidateWorkflowAttributesDeleteRequest(t *testing.T) {
Workflow: "workflow",
}))
}

func TestValidateListAllMatchableAttributesRequest(t *testing.T) {
err := ValidateListAllMatchableAttributesRequest(admin.ListMatchableAttributesRequest{
ResourceType: 44,
})
assert.EqualError(t, err, "invalid value for resource_type")

err = ValidateListAllMatchableAttributesRequest(admin.ListMatchableAttributesRequest{
ResourceType: admin.MatchableResource_EXECUTION_QUEUE,
})
assert.Nil(t, err)
}
3 changes: 3 additions & 0 deletions pkg/manager/interfaces/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ type ResourceInterface interface {
*admin.ProjectDomainAttributesGetResponse, error)
DeleteProjectDomainAttributes(ctx context.Context, request admin.ProjectDomainAttributesDeleteRequest) (
*admin.ProjectDomainAttributesDeleteResponse, error)

ListAll(ctx context.Context, request admin.ListMatchableAttributesRequest) (
*admin.ListMatchableAttributesResponse, error)
}

// TODO we can move this to flyteidl, once we are exposing an endpoint
Expand Down
11 changes: 11 additions & 0 deletions pkg/manager/mocks/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ type GetProjectDomainFunc func(ctx context.Context, request admin.ProjectDomainA
*admin.ProjectDomainAttributesGetResponse, error)
type DeleteProjectDomainFunc func(ctx context.Context, request admin.ProjectDomainAttributesDeleteRequest) (
*admin.ProjectDomainAttributesDeleteResponse, error)
type ListResourceFunc func(ctx context.Context, request admin.ListMatchableAttributesRequest) (
*admin.ListMatchableAttributesResponse, error)

type MockResourceManager struct {
updateProjectDomainFunc UpdateProjectDomainFunc
GetFunc GetProjectDomainFunc
DeleteFunc DeleteProjectDomainFunc
ListFunc ListResourceFunc
}

func (m *MockResourceManager) GetResource(ctx context.Context, request interfaces.ResourceRequest) (*interfaces.ResourceResponse, error) {
Expand Down Expand Up @@ -70,3 +73,11 @@ func (m *MockResourceManager) DeleteProjectDomainAttributes(
}
return nil, nil
}

func (m *MockResourceManager) ListAll(ctx context.Context, request admin.ListMatchableAttributesRequest) (
*admin.ListMatchableAttributesResponse, error) {
if m.ListFunc != nil {
return m.ListFunc(ctx, request)
}
return nil, nil
}
17 changes: 16 additions & 1 deletion pkg/repositories/gormimpl/resource_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ type ResourceRepo struct {
metrics gormMetrics
}

const priorityDescending = "priority desc"

/*
The data in the Resource repo maps to the following rules:
* Domain and ResourceType can never be empty.
Expand Down Expand Up @@ -103,7 +105,7 @@ func (r *ResourceRepo) Get(ctx context.Context, ID interfaces.ResourceID) (model
}

tx := r.db.Where(txWhereClause, ID.ResourceType, ID.Domain, project, workflow, launchPlan)
tx.Order("priority desc").First(&resources)
tx.Order(priorityDescending).First(&resources)
timer.Stop()

if tx.Error != nil {
Expand Down Expand Up @@ -140,6 +142,19 @@ func (r *ResourceRepo) GetRaw(ctx context.Context, ID interfaces.ResourceID) (mo
return model, nil
}

func (r *ResourceRepo) ListAll(ctx context.Context, resourceType string) ([]models.Resource, error) {
var resources []models.Resource
timer := r.metrics.ListDuration.Start()

tx := r.db.Where(&models.Resource{ResourceType: resourceType}).Order(priorityDescending).Find(&resources)
timer.Stop()

if tx.Error != nil {
return nil, r.errorTransformer.ToFlyteAdminError(tx.Error)
}
return resources, nil
}

func (r *ResourceRepo) Delete(ctx context.Context, ID interfaces.ResourceID) error {
var tx *gorm.DB
r.metrics.DeleteDuration.Time(func() {
Expand Down
38 changes: 35 additions & 3 deletions pkg/repositories/gormimpl/resource_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"github.com/stretchr/testify/assert"
)

const resourceTestWorkflowName = "workflow"

func TestCreateWorkflowAttributes(t *testing.T) {
resourceRepo := NewResourceRepo(GetDbForTest(t), errors.NewTestErrorTransformer(), mockScope.NewTestScope())
GlobalMock := mocket.Catcher.Reset()
Expand All @@ -25,7 +27,7 @@ func TestCreateWorkflowAttributes(t *testing.T) {
err := resourceRepo.CreateOrUpdate(context.Background(), models.Resource{
Project: "project",
Domain: "domain",
Workflow: "workflow",
Workflow: resourceTestWorkflowName,
ResourceType: "resource",
Priority: models.ResourcePriorityLaunchPlanLevel,
Attributes: []byte("attrs"),
Expand All @@ -41,7 +43,7 @@ func TestGetWorkflowAttributes(t *testing.T) {
response := make(map[string]interface{})
response["project"] = "project"
response["domain"] = "domain"
response["workflow"] = "workflow"
response["workflow"] = resourceTestWorkflowName
response["resource_type"] = "resource-type"
response["attributes"] = []byte("attrs")

Expand Down Expand Up @@ -98,7 +100,7 @@ func TestGetRawWorkflowAttributes(t *testing.T) {
response := make(map[string]interface{})
response[project] = project
response[domain] = domain
response["workflow"] = "workflow"
response["workflow"] = resourceTestWorkflowName
response["resource_type"] = "resource"
response["launch_plan"] = "launch_plan"
response["attributes"] = []byte("attrs")
Expand Down Expand Up @@ -135,3 +137,33 @@ func TestDeleteWorkflowAttributes(t *testing.T) {
assert.Nil(t, err)
assert.True(t, fakeResponse.Triggered)
}

func TestListAll(t *testing.T) {
resourceRepo := NewResourceRepo(GetDbForTest(t), errors.NewTestErrorTransformer(), mockScope.NewTestScope())
GlobalMock := mocket.Catcher.Reset()
GlobalMock.Logging = true

query := GlobalMock.NewMock()

response := make(map[string]interface{})
response[project] = project
response[domain] = domain
response["workflow"] = resourceTestWorkflowName
response["resource_type"] = "resource"
response["launch_plan"] = "launch_plan"
response["attributes"] = []byte("attrs")

fakeResponse := query.WithQuery(`SELECT * FROM "resources" WHERE "resources"."deleted_at" IS NULL AND ` +
`(("resources"."resource_type" = resource)) ORDER BY priority desc`).WithReply(
[]map[string]interface{}{response})
output, err := resourceRepo.ListAll(context.Background(), "resource")
assert.Nil(t, err)
assert.Len(t, output, 1)
assert.Equal(t, project, output[0].Project)
assert.Equal(t, domain, output[0].Domain)
assert.Equal(t, "workflow", output[0].Workflow)
assert.Equal(t, "launch_plan", output[0].LaunchPlan)
assert.Equal(t, "resource", output[0].ResourceType)
assert.Equal(t, []byte("attrs"), output[0].Attributes)
assert.True(t, fakeResponse.Triggered)
}
2 changes: 2 additions & 0 deletions pkg/repositories/interfaces/resource_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type ResourceRepoInterface interface {
Get(ctx context.Context, ID ResourceID) (models.Resource, error)
// Returns a matching Type model.
GetRaw(ctx context.Context, ID ResourceID) (models.Resource, error)
// Lists all resources
ListAll(ctx context.Context, resourceType string) ([]models.Resource, error)
// Deletes a matching Type model when it exists.
Delete(ctx context.Context, ID ResourceID) error
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/repositories/mocks/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ import (
type CreateOrUpdateResourceFunction func(ctx context.Context, input models.Resource) error
type GetResourceFunction func(ctx context.Context, ID interfaces.ResourceID) (
models.Resource, error)
type ListAllResourcesFunction func(ctx context.Context, resourceType string) ([]models.Resource, error)
type DeleteResourceFunction func(ctx context.Context, ID interfaces.ResourceID) error

type MockResourceRepo struct {
CreateOrUpdateFunction CreateOrUpdateResourceFunction
GetFunction GetResourceFunction
DeleteFunction DeleteResourceFunction
ListAllFunction ListAllResourcesFunction
}

func (r *MockResourceRepo) CreateOrUpdate(ctx context.Context, input models.Resource) error {
Expand All @@ -41,6 +43,13 @@ func (r *MockResourceRepo) GetRaw(ctx context.Context, ID interfaces.ResourceID)
return models.Resource{}, nil
}

func (r *MockResourceRepo) ListAll(ctx context.Context, resourceType string) ([]models.Resource, error) {
if r.ListAllFunction != nil {
return r.ListAllFunction(ctx, resourceType)
}
return []models.Resource{}, nil
}

func (r *MockResourceRepo) Delete(ctx context.Context, ID interfaces.ResourceID) error {
if r.DeleteFunction != nil {
return r.DeleteFunction(ctx, ID)
Expand Down
Loading

0 comments on commit 15d1fa0

Please sign in to comment.