Skip to content

Commit

Permalink
fix(backend): updated the argo version too 2.7.7. Fixes #4392 (#4498)
Browse files Browse the repository at this point in the history
* updated the version

* updated the serializer

* fixed test

* fixed some more  changes

* tested to update versions of k8 packages

* reverted package update

* change in API

* fixed dependencies, need to fix broken tests now

* updated fake client and fixed test due to updates in timestam.timestamp

* missed  to update fake client pod

* fixed issue in controller

* tested to update

* updated

* updated controller viewer

* updates to fix go mod vendor

* Updated the client

* updated the golang versions

* missed one docker file update, from 1.11 -> 1.13

* testing to fixe persistinace agent issues

* Updated after feedback

Co-authored-by: Niklas hansson <niklashansson@Niklass-MacBook-Pro.local>
  • Loading branch information
NikeNano and Niklas hansson committed Oct 23, 2020
1 parent 000da72 commit d7793af
Show file tree
Hide file tree
Showing 30 changed files with 834 additions and 257 deletions.
2 changes: 1 addition & 1 deletion backend/Dockerfile.cacheserver
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Dockerfile for building the source code of cache_server
FROM golang:1.11-alpine3.7 as builder
FROM golang:1.13.15-alpine3.12 as builder

RUN apk update && apk upgrade && \
apk add --no-cache bash git openssh gcc musl-dev
Expand Down
2 changes: 1 addition & 1 deletion backend/Dockerfile.persistenceagent
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.11-alpine3.7 as builder
FROM golang:1.13.15-alpine3.12 as builder

WORKDIR /go/src/github.com/kubeflow/pipelines
COPY . .
Expand Down
2 changes: 1 addition & 1 deletion backend/Dockerfile.scheduledworkflow
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM golang:1.11-alpine3.7 as builder
FROM golang:1.13.15-alpine3.12 as builder

WORKDIR /go/src/github.com/kubeflow/pipelines
COPY . .
Expand Down
10 changes: 5 additions & 5 deletions backend/src/agent/persistence/persistence_agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ type PersistenceAgent struct {

// NewPersistenceAgent returns a new persistence agent.
func NewPersistenceAgent(
swfInformerFactory swfinformers.SharedInformerFactory,
workflowInformerFactory workflowinformers.SharedInformerFactory,
pipelineClient *client.PipelineClient,
time util.TimeInterface) *PersistenceAgent {
swfInformerFactory swfinformers.SharedInformerFactory,
workflowInformerFactory workflowinformers.SharedInformerFactory,
pipelineClient *client.PipelineClient,
time util.TimeInterface) *PersistenceAgent {
// obtain references to shared informers
swfInformer := swfInformerFactory.Scheduledworkflow().V1beta1().ScheduledWorkflows()
workflowInformer := workflowInformerFactory.Argoproj().V1alpha1().Workflows()
Expand All @@ -62,7 +62,7 @@ func NewPersistenceAgent(
swfWorker := worker.NewPersistenceWorker(time, swfregister.Kind, swfInformer.Informer(), true,
worker.NewScheduledWorkflowSaver(swfClient, pipelineClient))

workflowWorker := worker.NewPersistenceWorker(time, workflowregister.Kind,
workflowWorker := worker.NewPersistenceWorker(time, workflowregister.WorkflowKind,
workflowInformer.Informer(), true,
worker.NewWorkflowSaver(workflowClient, pipelineClient, ttlSecondsAfterWorkflowFinish))

Expand Down
13 changes: 12 additions & 1 deletion backend/src/apiserver/client/pod_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ package client

import (
"errors"

"github.com/golang/glog"
corev1 "k8s.io/api/core/v1"
"k8s.io/api/policy/v1beta1"
"k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/rest"
Expand All @@ -14,6 +15,16 @@ import (
type FakePodClient struct {
}

func (FakePodClient) GetEphemeralContainers(string, v1.GetOptions) (*corev1.EphemeralContainers, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
}

func (FakePodClient) UpdateEphemeralContainers(string, *corev1.EphemeralContainers) (*corev1.EphemeralContainers, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
}

func (FakePodClient) Create(*corev1.Pod) (*corev1.Pod, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
Expand Down
2 changes: 1 addition & 1 deletion backend/src/apiserver/resource/resource_manager_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func formulateRetryWorkflow(wf *util.Workflow) (*util.Workflow, []string, error)
// Do not allow retry of workflows with pods in Running/Pending phase
return nil, nil, util.NewInternalServerError(
errors.New("workflow cannot be retried"),
"Workflow cannot be retried with node %s in %s phase", node, node.Phase)
"Workflow cannot be retried with node %s in %s phase", node.ID, node.Phase)
}
if node.Type == wfv1.NodeTypePod {
podsToDelete = append(podsToDelete, node.ID)
Expand Down
9 changes: 5 additions & 4 deletions backend/src/apiserver/resource/resource_manager_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,8 @@ spec:
arguments: {}
entrypoint: rand-fail-dag
templates:
- dag:
- arguments: {}
dag:
tasks:
- arguments: {}
name: A
Expand All @@ -277,10 +278,10 @@ spec:
metadata: {}
name: rand-fail-dag
outputs: {}
- container:
- arguments: {}
container:
args:
- import random; import sys; exit_code = random.choice([0, 0, 1]); print('exiting
with code {}'.format(exit_code)); sys.exit(exit_code)
- import random; import sys; exit_code = random.choice([0, 0, 1]); print('exiting with code {}'.format(exit_code)); sys.exit(exit_code)
command:
- python
- -c
Expand Down
3 changes: 2 additions & 1 deletion backend/src/apiserver/server/experiment_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/golang/protobuf/ptypes/timestamp"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
api "github.com/kubeflow/pipelines/backend/api/go_client"
"github.com/kubeflow/pipelines/backend/src/apiserver/client"
"github.com/kubeflow/pipelines/backend/src/apiserver/common"
Expand Down Expand Up @@ -394,7 +395,7 @@ func TestListExperiment_Multiuser(t *testing.T) {
} else {
if err != nil {
t.Errorf("TestListExperiment_Multiuser(%v) expect no error but got %v", tc.name, err)
} else if !cmp.Equal(tc.expectedExperiments, response.Experiments) {
} else if !cmp.Equal(tc.expectedExperiments, response.Experiments, cmpopts.IgnoreFields(api.Experiment{}, "CreatedAt")) {
t.Errorf("TestListExperiment_Multiuser(%v) expect (%+v) but got (%+v)", tc.name, tc.expectedExperiments, response.Experiments)
}
}
Expand Down
5 changes: 4 additions & 1 deletion backend/src/apiserver/server/job_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/golang/protobuf/ptypes/timestamp"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
api "github.com/kubeflow/pipelines/backend/api/go_client"
"github.com/kubeflow/pipelines/backend/src/apiserver/client"
"github.com/kubeflow/pipelines/backend/src/apiserver/common"
Expand Down Expand Up @@ -426,7 +427,9 @@ func TestListJobs_Multiuser(t *testing.T) {
} else {
if err != nil {
t.Errorf("TestListJobs_Multiuser(%v) expect no error but got %v", tc.name, err)
} else if !cmp.Equal(tc.expectedJobs, response.Jobs) {
} else if !cmp.Equal(tc.expectedJobs, response.Jobs, cmpopts.IgnoreFields(api.Job{}, "Trigger"),
cmpopts.IgnoreFields(api.Run{}, "CreatedAt"), cmpopts.IgnoreFields(api.Job{}, "UpdatedAt"),
cmpopts.IgnoreFields(api.Job{}, "CreatedAt")) {
t.Errorf("TestListJobs_Multiuser(%v) expect (%+v) but got (%+v)", tc.name, tc.expectedJobs, response.Jobs)
}
}
Expand Down
4 changes: 3 additions & 1 deletion backend/src/apiserver/server/run_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/golang/protobuf/ptypes/timestamp"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
api "github.com/kubeflow/pipelines/backend/api/go_client"
"github.com/kubeflow/pipelines/backend/src/apiserver/common"
"github.com/kubeflow/pipelines/backend/src/common/util"
Expand Down Expand Up @@ -375,7 +376,8 @@ func TestListRuns_Multiuser(t *testing.T) {
} else {
if err != nil {
t.Errorf("TestListRuns_Multiuser(%v) expect no error but got %v", tc.name, err)
} else if !cmp.Equal(tc.expectedRuns, response.Runs) {
} else if !cmp.Equal(tc.expectedRuns, response.Runs, cmpopts.IgnoreFields(api.Run{}, "CreatedAt"),
cmpopts.IgnoreFields(api.Run{}, "ScheduledAt"), cmpopts.IgnoreFields(api.Run{}, "FinishedAt")) {
t.Errorf("TestListRuns_Multiuser(%v) expect (%+v) but got (%+v)", tc.name, tc.expectedRuns, response.Runs)
}
}
Expand Down
10 changes: 10 additions & 0 deletions backend/src/cache/client/pod_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,16 @@ type FakePodClient struct {
patchIsCalled bool
}

func (FakePodClient) GetEphemeralContainers(string, v1.GetOptions) (*corev1.EphemeralContainers, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
}

func (FakePodClient) UpdateEphemeralContainers(string, *corev1.EphemeralContainers) (*corev1.EphemeralContainers, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
}

func (FakePodClient) Create(*corev1.Pod) (*corev1.Pod, error) {
glog.Error("This fake method is not yet implemented.")
return nil, nil
Expand Down
2 changes: 1 addition & 1 deletion backend/src/common/util/workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func TestToStringForStore(t *testing.T) {
},
})
assert.Equal(t,
"{\"metadata\":{\"name\":\"WORKFLOW_NAME\",\"creationTimestamp\":null},\"spec\":{\"templates\":null,\"entrypoint\":\"\",\"arguments\":{}},\"status\":{\"startedAt\":null,\"finishedAt\":null}}",
"{\"metadata\":{\"name\":\"WORKFLOW_NAME\",\"creationTimestamp\":null},\"spec\":{\"templates\":null,\"arguments\":{}},\"status\":{\"startedAt\":null,\"finishedAt\":null}}",
workflow.ToStringForStore())
}

Expand Down
3 changes: 1 addition & 2 deletions backend/src/crd/controller/viewer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ func main() {
log.Fatal(err)
}

_, err = builder.SimpleController().
WithManager(mgr).
_, err = builder.ControllerManagedBy(mgr).
ForType(&viewerV1beta1.Viewer{}).
Owns(&appsv1.Deployment{}).
Owns(&corev1.Service{}).
Expand Down
2 changes: 1 addition & 1 deletion backend/src/crd/controller/viewer/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func serviceFrom(v *viewerV1beta1.Viewer, deploymentName string) *corev1.Service
func (r *Reconciler) maybeDeleteOldestViewer(t viewerV1beta1.ViewerType, namespace string) error {
list := &viewerV1beta1.ViewerList{}

if err := r.Client.List(context.Background(), &client.ListOptions{Namespace: namespace}, list); err != nil {
if err := r.Client.List(context.Background(), list, &client.ListOptions{Namespace: namespace}); err != nil {
return fmt.Errorf("failed to list viewers: %v", err)
}

Expand Down
21 changes: 12 additions & 9 deletions backend/src/crd/controller/viewer/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestMain(m *testing.M) {
func getDeployments(t *testing.T, c client.Client) []*appsv1.Deployment {
dplList := &appsv1.DeploymentList{}

if err := c.List(context.Background(), &client.ListOptions{}, dplList); err != nil {
if err := c.List(context.Background(), dplList, &client.ListOptions{}); err != nil {
t.Fatalf("Failed to list deployments from Fake client: %v", err)
}

Expand All @@ -63,7 +63,7 @@ func getDeployments(t *testing.T, c client.Client) []*appsv1.Deployment {
func getServices(t *testing.T, c client.Client) []*corev1.Service {
svcList := &corev1.ServiceList{}

if err := c.List(context.Background(), &client.ListOptions{}, svcList); err != nil {
if err := c.List(context.Background(), svcList, &client.ListOptions{}); err != nil {
t.Fatalf("Failed to list services with fake client: %v", err)
}

Expand All @@ -78,7 +78,7 @@ func getServices(t *testing.T, c client.Client) []*corev1.Service {
func getViewers(t *testing.T, c client.Client) []*viewerV1beta1.Viewer {
list := &viewerV1beta1.ViewerList{}

if err := c.List(context.Background(), &client.ListOptions{}, list); err != nil {
if err := c.List(context.Background(), list, &client.ListOptions{}); err != nil {
t.Fatalf("Failed to list viewers with fake client: %v", err)
}

Expand Down Expand Up @@ -150,8 +150,9 @@ func TestReconcile_EachViewerCreatesADeployment(t *testing.T) {

wantDpls := []*appsv1.Deployment{{
ObjectMeta: metav1.ObjectMeta{
Name: "viewer-123-deployment",
Namespace: "kubeflow",
Name: "viewer-123-deployment",
Namespace: "kubeflow",
ResourceVersion: "1",
OwnerReferences: []metav1.OwnerReference{{
APIVersion: "kubeflow.org/v1beta1",
Name: "viewer-123",
Expand Down Expand Up @@ -249,8 +250,9 @@ func TestReconcile_ViewerUsesSpecifiedVolumeMountsForDeployment(t *testing.T) {

wantDpls := []*appsv1.Deployment{{
ObjectMeta: metav1.ObjectMeta{
Name: "viewer-123-deployment",
Namespace: "kubeflow",
Name: "viewer-123-deployment",
Namespace: "kubeflow",
ResourceVersion: "1",
OwnerReferences: []metav1.OwnerReference{{
APIVersion: "kubeflow.org/v1beta1",
Name: "viewer-123",
Expand Down Expand Up @@ -337,8 +339,9 @@ func TestReconcile_EachViewerCreatesAService(t *testing.T) {

want := []*v1.Service{{
ObjectMeta: metav1.ObjectMeta{
Name: "viewer-123-service",
Namespace: "kubeflow",
Name: "viewer-123-service",
Namespace: "kubeflow",
ResourceVersion: "1",
Annotations: map[string]string{
"getambassador.io/config": "\n---\n" +
"apiVersion: ambassador/v0\n" +
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 7 additions & 8 deletions backend/src/crd/pkg/client/clientset/versioned/clientset.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit d7793af

Please sign in to comment.