Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete dev resources only when stopping Dev session #5751

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ In the above example, three things have happened:
* `odo` has port-forwarded your application for local accessability
* `odo` will watch for changes in the current directory and rebuild the application when changes are detected

You can press Ctrl-c at any time to terminate the development session. The command can take a few moment to terminate, as it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be: "can take a moment" or "can take a few moments".
But maybe a native English speaker could correct me? @dharmit ? ^^

Suggested change
You can press Ctrl-c at any time to terminate the development session. The command can take a few moment to terminate, as it
You can press Ctrl-c at any time to terminate the development session. The command can take a few moments to terminate, as it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did notice it, but didn't bring it up. However, I completely agree with your take on this.

will first delete all resources deployed into the cluster for this session before terminating.

## Devfile (Advanced Usage)


Expand Down
84 changes: 45 additions & 39 deletions pkg/component/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,54 +87,60 @@ func references(list []unstructured.Unstructured, ownerRef metav1.OwnerReference
}

// ListResourcesToDeleteFromDevfile parses all the devfile components and returns a list of resources that are present on the cluster and can be deleted
func (do DeleteComponentClient) ListResourcesToDeleteFromDevfile(devfileObj parser.DevfileObj, appName string) (isInnerLoopDeployed bool, resources []unstructured.Unstructured, err error) {
// Inner Loop
// Fetch the deployment of the devfile component
componentName := devfileObj.GetMetadataName()
var deploymentName string
deploymentName, err = util.NamespaceKubernetesObject(componentName, appName)
if err != nil {
return isInnerLoopDeployed, resources, fmt.Errorf("failed to get the resource %q name for component %q; cause: %w", kclient.DeploymentKind, deploymentName, err)
}
func (do DeleteComponentClient) ListResourcesToDeleteFromDevfile(devfileObj parser.DevfileObj, appName string, mode string) (isInnerLoopDeployed bool, resources []unstructured.Unstructured, err error) {

if mode == odolabels.ComponentDevMode || mode == odolabels.ComponentAnyMode {
// Inner Loop
// Fetch the deployment of the devfile component
componentName := devfileObj.GetMetadataName()
var deploymentName string
deploymentName, err = util.NamespaceKubernetesObject(componentName, appName)
if err != nil {
return isInnerLoopDeployed, resources, fmt.Errorf("failed to get the resource %q name for component %q; cause: %w", kclient.DeploymentKind, componentName, err)
}

deployment, err := do.kubeClient.GetDeploymentByName(deploymentName)
if err != nil && !kerrors.IsNotFound(err) {
return isInnerLoopDeployed, resources, err
}
deployment, err := do.kubeClient.GetDeploymentByName(deploymentName)
if err != nil && !kerrors.IsNotFound(err) {
return isInnerLoopDeployed, resources, err
}

// if the deployment is found on the cluster,
// then convert it to unstructured.Unstructured object so that it can be appended to resources;
// else continue to outer loop
if deployment.Name != "" {
isInnerLoopDeployed = true
var unstructuredDeploy unstructured.Unstructured
unstructuredDeploy, err = kclient.ConvertK8sResourceToUnstructured(deployment)
if err != nil {
return isInnerLoopDeployed, resources, fmt.Errorf("failed to parse the resource %q: %q; cause: %w", kclient.DeploymentKind, deploymentName, err)
// if the deployment is found on the cluster,
// then convert it to unstructured.Unstructured object so that it can be appended to resources;
// else continue to outer loop
if deployment.Name != "" {
isInnerLoopDeployed = true
var unstructuredDeploy unstructured.Unstructured
unstructuredDeploy, err = kclient.ConvertK8sResourceToUnstructured(deployment)
if err != nil {
return isInnerLoopDeployed, resources, fmt.Errorf("failed to parse the resource %q: %q; cause: %w", kclient.DeploymentKind, deploymentName, err)
}
resources = append(resources, unstructuredDeploy)
}
resources = append(resources, unstructuredDeploy)
}

// Outer Loop
// Parse the devfile for outerloop K8s resources
localResources, err := libdevfile.ListKubernetesComponents(devfileObj, filepath.Dir(devfileObj.Ctx.GetAbsPath()))
if err != nil {
return isInnerLoopDeployed, resources, fmt.Errorf("failed to gather resources for deletion: %w", err)
}
for _, lr := range localResources {
var gvr *meta.RESTMapping
gvr, err = do.kubeClient.GetRestMappingFromUnstructured(lr)
if mode == odolabels.ComponentDeployMode || mode == odolabels.ComponentAnyMode {
// Outer Loop
// Parse the devfile for outerloop K8s resources
localResources, err := libdevfile.ListKubernetesComponents(devfileObj, filepath.Dir(devfileObj.Ctx.GetAbsPath()))
if err != nil {
continue
return isInnerLoopDeployed, resources, fmt.Errorf("failed to gather resources for deletion: %w", err)
}
// Try to fetch the resource from the cluster; if it exists, append it to the resources list
var cr *unstructured.Unstructured
cr, err = do.kubeClient.GetDynamicResource(gvr.Resource, lr.GetName())
if err != nil {
continue
for _, lr := range localResources {
var gvr *meta.RESTMapping
gvr, err = do.kubeClient.GetRestMappingFromUnstructured(lr)
if err != nil {
continue
}
// Try to fetch the resource from the cluster; if it exists, append it to the resources list
var cr *unstructured.Unstructured
cr, err = do.kubeClient.GetDynamicResource(gvr.Resource, lr.GetName())
if err != nil {
continue
}
resources = append(resources, *cr)
}
resources = append(resources, *cr)
}

return isInnerLoopDeployed, resources, nil
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/component/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"

"github.com/redhat-developer/odo/pkg/kclient"
"github.com/redhat-developer/odo/pkg/labels"
odolabels "github.com/redhat-developer/odo/pkg/labels"
odoTestingUtil "github.com/redhat-developer/odo/pkg/testingutil"
"github.com/redhat-developer/odo/pkg/util"
Expand Down Expand Up @@ -465,7 +466,7 @@ func TestDeleteComponentClient_ListResourcesToDeleteFromDevfile(t *testing.T) {
do := DeleteComponentClient{
kubeClient: tt.fields.kubeClient(ctrl),
}
gotIsInnerLoopDeployed, gotResources, err := do.ListResourcesToDeleteFromDevfile(tt.args.devfileObj, tt.args.appName)
gotIsInnerLoopDeployed, gotResources, err := do.ListResourcesToDeleteFromDevfile(tt.args.devfileObj, tt.args.appName, labels.ComponentAnyMode)
if (err != nil) != tt.wantErr {
t.Errorf("ListResourcesToDeleteFromDevfile() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down
3 changes: 2 additions & 1 deletion pkg/component/delete/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ type Client interface {
ExecutePreStopEvents(devfileObj parser.DevfileObj, appName string) error
// ListResourcesToDeleteFromDevfile parses all the devfile components and returns a list of resources that are present on the cluster that can be deleted,
// and a bool that indicates if the devfile component has been pushed to the innerloop
ListResourcesToDeleteFromDevfile(devfileObj parser.DevfileObj, appName string) (bool, []unstructured.Unstructured, error)
// the mode indicates which component to list, either Dev, Deploy or Any (using constant labels.Component*Mode)
ListResourcesToDeleteFromDevfile(devfileObj parser.DevfileObj, appName string, mode string) (bool, []unstructured.Unstructured, error)
}
8 changes: 4 additions & 4 deletions pkg/component/delete/mock.go

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

3 changes: 2 additions & 1 deletion pkg/odo/cli/delete/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
ktemplates "k8s.io/kubectl/pkg/util/templates"

"github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/log"
"github.com/redhat-developer/odo/pkg/odo/cli/ui"
"github.com/redhat-developer/odo/pkg/odo/cmdline"
Expand Down Expand Up @@ -124,7 +125,7 @@ func (o *ComponentOptions) deleteDevfileComponent() error {
appName := "app"

log.Info("Searching resources to delete, please wait...")
isInnerLoopDeployed, devfileResources, err := o.clientset.DeleteClient.ListResourcesToDeleteFromDevfile(devfileObj, appName)
isInnerLoopDeployed, devfileResources, err := o.clientset.DeleteClient.ListResourcesToDeleteFromDevfile(devfileObj, appName, labels.ComponentAnyMode)
if err != nil {
return err
}
Expand Down
11 changes: 6 additions & 5 deletions pkg/odo/cli/delete/component/component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
_delete "github.com/redhat-developer/odo/pkg/component/delete"
"github.com/redhat-developer/odo/pkg/envinfo"
"github.com/redhat-developer/odo/pkg/kclient"
"github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/odo/cmdline"
"github.com/redhat-developer/odo/pkg/odo/genericclioptions"
"github.com/redhat-developer/odo/pkg/odo/genericclioptions/clientset"
Expand Down Expand Up @@ -129,7 +130,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) {
name: "deleting a component with access to devfile",
deleteClient: func(ctrl *gomock.Controller) _delete.Client {
deleteClient := _delete.NewMockClient(ctrl)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName).Return(true, resources, nil)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName, labels.ComponentAnyMode).Return(true, resources, nil)
deleteClient.EXPECT().ListClusterResourcesToDelete(compName, projectName).Return(resources, nil)
deleteClient.EXPECT().ExecutePreStopEvents(gomock.Any(), gomock.Any()).Return(nil)
deleteClient.EXPECT().DeleteResources(resources, false).Return([]unstructured.Unstructured{})
Expand All @@ -144,7 +145,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) {
name: "deleting a component should not fail even if ExecutePreStopEvents fails",
deleteClient: func(ctrl *gomock.Controller) _delete.Client {
deleteClient := _delete.NewMockClient(ctrl)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName).Return(true, resources, nil)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName, labels.ComponentAnyMode).Return(true, resources, nil)
deleteClient.EXPECT().ListClusterResourcesToDelete(compName, projectName).Return(resources, nil)
deleteClient.EXPECT().ExecutePreStopEvents(gomock.Any(), appName).Return(errors.New("some error"))
deleteClient.EXPECT().DeleteResources(resources, false).Return(nil)
Expand All @@ -159,7 +160,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) {
name: "deleting a component should fail if ListResourcesToDeleteFromDevfile fails",
deleteClient: func(ctrl *gomock.Controller) _delete.Client {
deleteClient := _delete.NewMockClient(ctrl)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName).Return(false, nil, errors.New("some error"))
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName, labels.ComponentAnyMode).Return(false, nil, errors.New("some error"))
return deleteClient
},
fields: fields{
Expand All @@ -171,7 +172,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) {
name: "deleting a component should be aborted if forceFlag is not passed",
deleteClient: func(ctrl *gomock.Controller) _delete.Client {
deleteClient := _delete.NewMockClient(ctrl)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName).Return(true, resources, nil)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName, labels.ComponentAnyMode).Return(true, resources, nil)
return deleteClient
},
fields: fields{
Expand All @@ -183,7 +184,7 @@ func TestComponentOptions_deleteDevfileComponent(t *testing.T) {
name: "nothing to delete",
deleteClient: func(ctrl *gomock.Controller) _delete.Client {
deleteClient := _delete.NewMockClient(ctrl)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName).Return(false, nil, nil)
deleteClient.EXPECT().ListResourcesToDeleteFromDevfile(gomock.Any(), appName, labels.ComponentAnyMode).Return(false, nil, nil)
return deleteClient
},
fields: fields{
Expand Down
4 changes: 2 additions & 2 deletions pkg/odo/cli/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func (o *DevOptions) Run(ctx context.Context) (err error) {

defer func() {
if err != nil {
_ = o.clientset.WatchClient.Cleanup(devFileObj, log.GetStdout())
_ = o.clientset.WatchClient.CleanupDevResources(devFileObj, log.GetStdout())
}
}()

Expand Down Expand Up @@ -254,7 +254,7 @@ func (o *DevOptions) Run(ctx context.Context) (err error) {
if o.noWatchFlag {
log.Finfof(log.GetStdout(), "\n"+watch.CtrlCMessage)
<-o.ctx.Done()
err = o.clientset.WatchClient.Cleanup(devFileObj, log.GetStdout())
err = o.clientset.WatchClient.CleanupDevResources(devFileObj, log.GetStdout())
} else {
d := Handler{}
err = o.clientset.DevClient.Watch(devFileObj, path, o.ignorePaths, o.out, &d, o.ctx, o.debugFlag)
Expand Down
4 changes: 2 additions & 2 deletions pkg/watch/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ type Client interface {
// WatchAndPush watches the component under the context directory and triggers Push if there are any changes
// It also listens on ctx's Done channel to trigger cleanup when indicated to do so
WatchAndPush(out io.Writer, parameters WatchParameters, ctx context.Context) error
// Cleanup deletes the component created using the devfileObj and writes any outputs to out
Cleanup(devfileObj parser.DevfileObj, out io.Writer) error
// CleanupDevResources deletes the component created using the devfileObj and writes any outputs to out
CleanupDevResources(devfileObj parser.DevfileObj, out io.Writer) error
}
12 changes: 6 additions & 6 deletions pkg/watch/mock.go

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

7 changes: 4 additions & 3 deletions pkg/watch/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/devfile/library/pkg/devfile/parser"
_delete "github.com/redhat-developer/odo/pkg/component/delete"
"github.com/redhat-developer/odo/pkg/labels"
"github.com/redhat-developer/odo/pkg/state"

"github.com/fsnotify/fsnotify"
Expand Down Expand Up @@ -213,7 +214,7 @@ func (o *WatchClient) WatchAndPush(out io.Writer, parameters WatchParameters, ct

printInfoMessage(out, parameters.Path)

return eventWatcher(ctx, watcher, parameters, out, evaluateFileChanges, processEvents, o.Cleanup)
return eventWatcher(ctx, watcher, parameters, out, evaluateFileChanges, processEvents, o.CleanupDevResources)
}

// eventWatcher loops till the context's Done channel indicates it to stop looping, at which point it performs cleanup.
Expand Down Expand Up @@ -358,9 +359,9 @@ func processEvents(changedFiles, deletedPaths []string, parameters WatchParamete
}
}

func (o *WatchClient) Cleanup(devfileObj parser.DevfileObj, out io.Writer) error {
func (o *WatchClient) CleanupDevResources(devfileObj parser.DevfileObj, out io.Writer) error {
fmt.Fprintln(out, "Cleaning resources, please wait")
isInnerLoopDeployed, resources, err := o.deleteClient.ListResourcesToDeleteFromDevfile(devfileObj, "app")
isInnerLoopDeployed, resources, err := o.deleteClient.ListResourcesToDeleteFromDevfile(devfileObj, "app", labels.ComponentDevMode)
if err != nil {
fmt.Fprintf(out, "failed to delete inner loop resources: %v", err)
return err
Expand Down
14 changes: 14 additions & 0 deletions tests/integration/devfile/cmd_devfile_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,20 @@ var _ = Describe("odo devfile deploy command tests", func() {
})

})

When("running and stopping odo dev", func() {
BeforeEach(func() {
session, _, _, _, err := helper.StartDevMode()
Expect(err).ShouldNot(HaveOccurred())
session.Stop()
session.WaitEnd()
})

It("should not delete the resources created with odo deploy", func() {
output := commonVar.CliRunner.Run("get", "deployment", "-n", commonVar.Project).Out.Contents()
Expect(string(output)).To(ContainSubstring(deploymentName))
})
})
})
})
}
Expand Down