From b268533240698e088bd48ea45ba63268f9a37c9f Mon Sep 17 00:00:00 2001 From: zychen5186 Date: Fri, 7 Jun 2024 00:09:48 -0700 Subject: [PATCH] Add domain matric, add test in flyteadmin and flytectl Signed-off-by: zychen5186 --- flyteadmin/pkg/rpc/adminservice/metrics.go | 11 ++++++ flyteadmin/pkg/rpc/adminservice/project.go | 4 +-- flyteadmin/tests/project_test.go | 14 ++++++++ flytectl/pkg/ext/domain_fetcher.go | 2 +- flytectl/pkg/ext/domain_fetcher_test.go | 35 +++++++++++++++++++ flytectl/pkg/ext/fetcher.go | 2 +- .../ext/mocks/admin_fetcher_ext_interface.go | 22 ++++++------ 7 files changed, 75 insertions(+), 15 deletions(-) create mode 100644 flytectl/pkg/ext/domain_fetcher_test.go diff --git a/flyteadmin/pkg/rpc/adminservice/metrics.go b/flyteadmin/pkg/rpc/adminservice/metrics.go index 5d0e89d93e0..f5904830b5c 100644 --- a/flyteadmin/pkg/rpc/adminservice/metrics.go +++ b/flyteadmin/pkg/rpc/adminservice/metrics.go @@ -55,6 +55,12 @@ type nodeExecutionEndpointMetrics struct { getDynamicNodeWorkflow util.RequestMetrics } +type domainEndpointMetrics struct { + scope promutils.Scope + + get util.RequestMetrics +} + type projectEndpointMetrics struct { scope promutils.Scope @@ -116,6 +122,7 @@ type AdminMetrics struct { launchPlanEndpointMetrics launchPlanEndpointMetrics namedEntityEndpointMetrics namedEntityEndpointMetrics nodeExecutionEndpointMetrics nodeExecutionEndpointMetrics + domainEndpointMetrics domainEndpointMetrics projectEndpointMetrics projectEndpointMetrics projectAttributesEndpointMetrics attributeEndpointMetrics projectDomainAttributesEndpointMetrics attributeEndpointMetrics @@ -172,6 +179,10 @@ func InitMetrics(adminScope promutils.Scope) AdminMetrics { listChildren: util.NewRequestMetrics(adminScope, "list_children_node_executions"), getDynamicNodeWorkflow: util.NewRequestMetrics(adminScope, "get_dynamic_node_workflow"), }, + domainEndpointMetrics: domainEndpointMetrics{ + scope: adminScope, + get: util.NewRequestMetrics(adminScope, "get_domain"), + }, projectEndpointMetrics: projectEndpointMetrics{ scope: adminScope, register: util.NewRequestMetrics(adminScope, "register_project"), diff --git a/flyteadmin/pkg/rpc/adminservice/project.go b/flyteadmin/pkg/rpc/adminservice/project.go index 0f0e0cf0fa9..46416919bf1 100644 --- a/flyteadmin/pkg/rpc/adminservice/project.go +++ b/flyteadmin/pkg/rpc/adminservice/project.go @@ -34,11 +34,11 @@ func (m *AdminService) GetDomains(ctx context.Context, request *admin.GetDomainR return nil, status.Errorf(codes.InvalidArgument, "Incorrect request, nil requests not allowed") } var response *admin.Domains - m.Metrics.projectEndpointMetrics.get.Time(func() { + m.Metrics.domainEndpointMetrics.get.Time(func() { response = m.ProjectManager.GetDomains(ctx, *request) }) - m.Metrics.projectEndpointMetrics.get.Success() + m.Metrics.domainEndpointMetrics.get.Success() return response, nil } diff --git a/flyteadmin/tests/project_test.go b/flyteadmin/tests/project_test.go index 0534ca1f3af..1aa051cffae 100644 --- a/flyteadmin/tests/project_test.go +++ b/flyteadmin/tests/project_test.go @@ -14,6 +14,20 @@ import ( "github.com/flyteorg/flyte/flytestdlib/utils" ) +func TestGetDomains(t *testing.T) { + ctx := context.Background() + client, conn := GetTestAdminServiceClient() + defer conn.Close() + + domains, err := client.GetDomains(ctx, &admin.GetDomainRequest{}) + assert.Nil(t, err) + assert.NotEmpty(t, domains.Domains) + for _, domain := range project.Domains { + assert.Contains(t, []string{"development", "domain", "staging", "production"}, domain.Id) + assert.Contains(t, []string{"development", "domain", "staging", "production"}, domain.Name) + } +} + func TestCreateProject(t *testing.T) { truncateAllTablesForTestingOnly() ctx := context.Background() diff --git a/flytectl/pkg/ext/domain_fetcher.go b/flytectl/pkg/ext/domain_fetcher.go index eb911707029..4519f0a9aec 100644 --- a/flytectl/pkg/ext/domain_fetcher.go +++ b/flytectl/pkg/ext/domain_fetcher.go @@ -6,7 +6,7 @@ import ( "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin" ) -func (a *AdminFetcherExtClient) GetDomain(ctx context.Context) (*admin.Domains, error) { +func (a *AdminFetcherExtClient) GetDomains(ctx context.Context) (*admin.Domains, error) { domains, err := a.AdminServiceClient().GetDomains(ctx, &admin.GetDomainRequest{}) if err != nil { return nil, err diff --git a/flytectl/pkg/ext/domain_fetcher_test.go b/flytectl/pkg/ext/domain_fetcher_test.go new file mode 100644 index 00000000000..b820c89121a --- /dev/null +++ b/flytectl/pkg/ext/domain_fetcher_test.go @@ -0,0 +1,35 @@ +package ext + +import ( + "testing" + + "github.com/flyteorg/flyte/flyteidl/clients/go/admin/mocks" + "github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +func TestAdminFetcherExtClient_GetDomains(t *testing.T) { + domain1 := &admin.Domain{ + Id: "development", + Name: "development", + } + domain2 := &admin.Domain{ + Id: "staging", + Name: "staging", + } + domain3 := &admin.Domain{ + Id: "production", + Name: "production", + } + domains := &admin.Domains{ + Domains: []*admin.Domain{domain1, domain2, domain3}, + } + + adminClient := new(mocks.AdminServiceClient) + adminFetcherExt := AdminFetcherExtClient{AdminClient: adminClient} + + adminClient.OnGetDomainsMatch(mock.Anything, mock.Anything).Return(domains, nil) + _, err := adminFetcherExt.GetDomains(ctx) + assert.Nil(t, err) +} diff --git a/flytectl/pkg/ext/fetcher.go b/flytectl/pkg/ext/fetcher.go index 76896e5e77a..72393e1a6bb 100644 --- a/flytectl/pkg/ext/fetcher.go +++ b/flytectl/pkg/ext/fetcher.go @@ -76,7 +76,7 @@ type AdminFetcherExtInterface interface { // GetProjectByID fetches a single project by its identifier. If project does not exist, an error will be returned GetProjectByID(ctx context.Context, projectID string) (*admin.Project, error) - GetDomain(ctx context.Context) (*admin.Domains, error) + GetDomains(ctx context.Context) (*admin.Domains, error) } // AdminFetcherExtClient is used for interacting with extended features used for fetching data from admin service diff --git a/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go b/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go index 2e52e12d4e0..4e0c62f0a9d 100644 --- a/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go +++ b/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go @@ -750,26 +750,26 @@ func (_m *AdminFetcherExtInterface) FetchWorkflowVersion(ctx context.Context, na return r0, r1 } -type AdminFetcherExtInterface_GetDomain struct { +type AdminFetcherExtInterface_GetDomains struct { *mock.Call } -func (_m AdminFetcherExtInterface_GetDomain) Return(_a0 *admin.Domains, _a1 error) *AdminFetcherExtInterface_GetDomain { - return &AdminFetcherExtInterface_GetDomain{Call: _m.Call.Return(_a0, _a1)} +func (_m AdminFetcherExtInterface_GetDomains) Return(_a0 *admin.Domains, _a1 error) *AdminFetcherExtInterface_GetDomains { + return &AdminFetcherExtInterface_GetDomains{Call: _m.Call.Return(_a0, _a1)} } -func (_m *AdminFetcherExtInterface) OnGetDomain(ctx context.Context) *AdminFetcherExtInterface_GetDomain { - c_call := _m.On("GetDomain", ctx) - return &AdminFetcherExtInterface_GetDomain{Call: c_call} +func (_m *AdminFetcherExtInterface) OnGetDomains(ctx context.Context) *AdminFetcherExtInterface_GetDomains { + c_call := _m.On("GetDomains", ctx) + return &AdminFetcherExtInterface_GetDomains{Call: c_call} } -func (_m *AdminFetcherExtInterface) OnGetDomainMatch(matchers ...interface{}) *AdminFetcherExtInterface_GetDomain { - c_call := _m.On("GetDomain", matchers...) - return &AdminFetcherExtInterface_GetDomain{Call: c_call} +func (_m *AdminFetcherExtInterface) OnGetDomainsMatch(matchers ...interface{}) *AdminFetcherExtInterface_GetDomains { + c_call := _m.On("GetDomains", matchers...) + return &AdminFetcherExtInterface_GetDomains{Call: c_call} } -// GetDomain provides a mock function with given fields: ctx -func (_m *AdminFetcherExtInterface) GetDomain(ctx context.Context) (*admin.Domains, error) { +// GetDomains provides a mock function with given fields: ctx +func (_m *AdminFetcherExtInterface) GetDomains(ctx context.Context) (*admin.Domains, error) { ret := _m.Called(ctx) var r0 *admin.Domains