Skip to content

Issue #139 - Application sync should delete 'unexpected' resources #144

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.

Sign up for GitHub

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

Merged
alexmt merged 3 commits into from
May 1, 2018
Merged

Issue #139 - Application sync should delete 'unexpected' resources #144

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b1925e0
Issue #139 - Application sync should delete 'unexpected' resources
Apr 30, 2018
723db87
Issue #139 - Add --prune flag to app sync and rollback commands
Apr 30, 2018
ed2364f
Apply reviewer notes: s/skipped/ignored/g; take pruned flag into cons…
Apr 30, 2018
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
12 changes: 10 additions & 2 deletions cmd/argocd/commands/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ func NewApplicationListCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
var (
revision string
prune bool
dryRun bool
)
var command = &cobra.Command{
Expand All @@ -366,6 +367,7 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
Name: appName,
DryRun: dryRun,
Revision: revision,
Prune: prune,
}
syncRes, err := appIf.Sync(context.Background(), &syncReq)
errors.CheckError(err)
Expand All @@ -379,6 +381,7 @@ func NewApplicationSyncCommand(clientOpts *argocdclient.ClientOptions) *cobra.Co
},
}
command.Flags().BoolVar(&dryRun, "dry-run", false, "Preview apply without affecting cluster")
command.Flags().BoolVar(&prune, "prune", false, "Allow deleting unexpected resources")
command.Flags().StringVar(&revision, "revision", "", "Sync to a specific revision. Preserves parameter overrides")
return command
}
Expand Down Expand Up @@ -460,6 +463,9 @@ func paramString(params []argoappv1.ComponentParameter) string {

// NewApplicationRollbackCommand returns a new instance of an `argocd app rollback` command
func NewApplicationRollbackCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
var (
prune bool
)
var command = &cobra.Command{
Use: "rollback",
Short: "Rollback application to a previous deployed version",
Expand Down Expand Up @@ -487,8 +493,9 @@ func NewApplicationRollbackCommand(clientOpts *argocdclient.ClientOptions) *cobr
log.Fatalf("Application '%s' does not have deployment id '%d' in history\n", app.ObjectMeta.Name, depID)
}
syncRes, err := appIf.Rollback(ctx, &application.ApplicationRollbackRequest{
Name: appName,
ID: int64(depID),
Name: appName,
ID: int64(depID),
Prune: prune,
})
errors.CheckError(err)
fmt.Printf("%s %s\n", appName, syncRes.Message)
Expand All @@ -500,5 +507,6 @@ func NewApplicationRollbackCommand(clientOpts *argocdclient.ClientOptions) *cobr
_ = w.Flush()
},
}
command.Flags().BoolVar(&prune, "prune", false, "Allow deleting unexpected resources")
return command
}
118 changes: 79 additions & 39 deletions server/application/application.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import (
"strings"
"time"

"encoding/json"

"github.com/argoproj/argo-cd/common"
"github.com/argoproj/argo-cd/controller"
appv1 "github.com/argoproj/argo-cd/pkg/apis/application/v1alpha1"
appclientset "github.com/argoproj/argo-cd/pkg/client/clientset/versioned"
"github.com/argoproj/argo-cd/reposerver"
Expand All @@ -15,7 +18,6 @@ import (
apirepository "github.com/argoproj/argo-cd/server/repository"
"github.com/argoproj/argo-cd/util"
argoutil "github.com/argoproj/argo-cd/util/argo"
"github.com/argoproj/argo-cd/util/diff"
"github.com/argoproj/argo-cd/util/git"
"github.com/argoproj/argo-cd/util/kube"
log "github.com/sirupsen/logrus"
Expand Down Expand Up @@ -43,6 +45,7 @@ type Server struct {
// TODO(jessesuen): move common cluster code to shared libraries
clusterService cluster.ClusterServiceServer
repoService apirepository.RepositoryServiceServer
appComparator controller.AppComparator
}

// NewServer returns a new instance of the Application service
Expand All @@ -61,6 +64,7 @@ func NewServer(
clusterService: clusterService,
repoClientset: repoClientset,
repoService: repoService,
appComparator: controller.NewKsonnetAppComparator(clusterService),
}
}

Expand Down Expand Up @@ -334,7 +338,7 @@ func (s *Server) PodLogs(q *PodLogsQuery, ws ApplicationService_PodLogsServer) e

// Sync syncs an application to its target state
func (s *Server) Sync(ctx context.Context, syncReq *ApplicationSyncRequest) (*ApplicationSyncResult, error) {
return s.deployAndPersistDeploymentInfo(ctx, syncReq.Name, syncReq.Revision, nil, syncReq.DryRun)
return s.deployAndPersistDeploymentInfo(ctx, syncReq.Name, syncReq.Revision, nil, syncReq.DryRun, syncReq.Prune)
}

func (s *Server) Rollback(ctx context.Context, rollbackReq *ApplicationRollbackRequest) (*ApplicationSyncResult, error) {
Expand All @@ -352,11 +356,11 @@ func (s *Server) Rollback(ctx context.Context, rollbackReq *ApplicationRollbackR
if deploymentInfo == nil {
return nil, status.Errorf(codes.InvalidArgument, "application %s does not have deployment with id %v", rollbackReq.Name, rollbackReq.ID)
}
return s.deployAndPersistDeploymentInfo(ctx, rollbackReq.Name, deploymentInfo.Revision, &deploymentInfo.ComponentParameterOverrides, rollbackReq.DryRun)
return s.deployAndPersistDeploymentInfo(ctx, rollbackReq.Name, deploymentInfo.Revision, &deploymentInfo.ComponentParameterOverrides, rollbackReq.DryRun, rollbackReq.Prune)
}

func (s *Server) deployAndPersistDeploymentInfo(
ctx context.Context, appName string, revision string, overrides *[]appv1.ComponentParameter, dryRun bool) (*ApplicationSyncResult, error) {
ctx context.Context, appName string, revision string, overrides *[]appv1.ComponentParameter, dryRun bool, prune bool) (*ApplicationSyncResult, error) {

log.Infof("Syncing application %s", appName)
app, err := s.Get(ctx, &ApplicationQuery{Name: appName})
Expand All @@ -372,7 +376,7 @@ func (s *Server) deployAndPersistDeploymentInfo(
app.Spec.Source.ComponentParameterOverrides = *overrides
}

res, manifest, err := s.deploy(ctx, app.Spec.Source, app.Spec.Destination, app.Name, dryRun)
res, manifest, err := s.deploy(ctx, app, dryRun, prune)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -470,43 +474,35 @@ func (s *Server) getRepo(ctx context.Context, repoURL string) *appv1.Repository

func (s *Server) deploy(
ctx context.Context,
source appv1.ApplicationSource,
destination *appv1.ApplicationDestination,
appLabel string,
dryRun bool) (*ApplicationSyncResult, *repository.ManifestResponse, error) {
app *appv1.Application,
dryRun bool,
prune bool) (*ApplicationSyncResult, *repository.ManifestResponse, error) {

repo := s.getRepo(ctx, source.RepoURL)
repo := s.getRepo(ctx, app.Spec.Source.RepoURL)
conn, repoClient, err := s.repoClientset.NewRepositoryClient()
if err != nil {
return nil, nil, err
}
defer util.Close(conn)
overrides := make([]*appv1.ComponentParameter, len(source.ComponentParameterOverrides))
if source.ComponentParameterOverrides != nil {
for i := range source.ComponentParameterOverrides {
item := source.ComponentParameterOverrides[i]
overrides := make([]*appv1.ComponentParameter, len(app.Spec.Source.ComponentParameterOverrides))
if app.Spec.Source.ComponentParameterOverrides != nil {
for i := range app.Spec.Source.ComponentParameterOverrides {
item := app.Spec.Source.ComponentParameterOverrides[i]
overrides[i] = &item
}
}

manifestInfo, err := repoClient.GenerateManifest(ctx, &repository.ManifestRequest{
Repo: repo,
Environment: source.Environment,
Path: source.Path,
Revision: source.TargetRevision,
Environment: app.Spec.Source.Environment,
Path: app.Spec.Source.Path,
Revision: app.Spec.Source.TargetRevision,
ComponentParameterOverrides: overrides,
AppLabel: appLabel,
AppLabel: app.Name,
})
if err != nil {
return nil, nil, err
}
server, namespace := argoutil.ResolveServerNamespace(destination, manifestInfo)

clst, err := s.clusterService.Get(ctx, &cluster.ClusterQuery{Server: server})
if err != nil {
return nil, nil, err
}
config := clst.RESTConfig()

targetObjs := make([]*unstructured.Unstructured, len(manifestInfo.Manifests))
for i, manifest := range manifestInfo.Manifests {
Expand All @@ -517,41 +513,85 @@ func (s *Server) deploy(
targetObjs[i] = obj
}

liveObjs, err := kube.GetLiveResources(config, targetObjs, namespace)
server, namespace := argoutil.ResolveServerNamespace(app.Spec.Destination, manifestInfo)

comparison, err := s.appComparator.CompareAppState(server, namespace, targetObjs, app)
if err != nil {
return nil, nil, err
}
diffResList, err := diff.DiffArray(targetObjs, liveObjs)

clst, err := s.clusterService.Get(ctx, &cluster.ClusterQuery{Server: server})
if err != nil {
return nil, nil, err
}
config := clst.RESTConfig()

var syncRes ApplicationSyncResult
syncRes.Resources = make([]*ResourceDetails, 0)
for i, diffRes := range diffResList.Diffs {
for _, resourceState := range comparison.Resources {
var liveObj, targetObj *unstructured.Unstructured

if resourceState.LiveState != "null" {
liveObj = &unstructured.Unstructured{}
err = json.Unmarshal([]byte(resourceState.LiveState), liveObj)
if err != nil {
return nil, nil, err
}
}

if resourceState.TargetState != "null" {
targetObj = &unstructured.Unstructured{}
err = json.Unmarshal([]byte(resourceState.TargetState), targetObj)
if err != nil {
return nil, nil, err
}
}

needsCreate := liveObj == nil
needsDelete := targetObj == nil

obj := targetObj
if obj == nil {
obj = liveObj
}
resDetails := ResourceDetails{
Name: targetObjs[i].GetName(),
Kind: targetObjs[i].GetKind(),
Name: obj.GetName(),
Kind: obj.GetKind(),
Namespace: namespace,
}
needsCreate := bool(liveObjs[i] == nil)
if !diffRes.Modified {

if resourceState.Status == appv1.ComparisonStatusSynced {
resDetails.Message = fmt.Sprintf("already synced")
} else if dryRun {
if needsCreate {
resDetails.Message = fmt.Sprintf("will create")
} else if needsDelete {
resDetails.Message = fmt.Sprintf("will delete")

@jessesuen jessesuen Apr 30, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs to take consideration the prune flag. Can we use the term "ignored" if we decide not to delete the resource?

@alexmt alexmt May 1, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

} else {
resDetails.Message = fmt.Sprintf("will update")
}
} else {
_, err := kube.ApplyResource(config, targetObjs[i], namespace)
if err != nil {
return nil, nil, err
}
if needsCreate {
resDetails.Message = fmt.Sprintf("created")
if needsDelete {
if prune {
err = kube.DeleteResource(config, liveObj, namespace)
if err != nil {
return nil, nil, err
}

resDetails.Message = fmt.Sprintf("deleted")
} else {
resDetails.Message = fmt.Sprintf("skipped (should be deleted)")

@jessesuen jessesuen Apr 30, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets use the term "ignored"

@alexmt alexmt May 1, 2018

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done

}
} else {
resDetails.Message = fmt.Sprintf("updated")
_, err := kube.ApplyResource(config, targetObj, namespace)
if err != nil {
return nil, nil, err
}
if needsCreate {
resDetails.Message = fmt.Sprintf("created")
} else {
resDetails.Message = fmt.Sprintf("updated")
}
}
}
syncRes.Resources = append(syncRes.Resources, &resDetails)
Expand Down
Loading