Skip to content

Persist parameters during deployment (Sync)#39

Merged
merenbach merged 33 commits intoargoproj:masterfrom
merenbach:recent-deployments
Mar 16, 2018
Merged

Persist parameters during deployment (Sync)#39
merenbach merged 33 commits intoargoproj:masterfrom
merenbach:recent-deployments

Conversation

@merenbach
Copy link
Contributor

Background

This addresses the first subtask in issue #27.

Implementation

  • Add new API command to request/receive JSON-encoded Ksonnet env params.
  • Read Ksonnet env params during a sync.
  • Store those env params into a Kubernetes custom resource definition (CRD).

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

I've noticed that controller override whole status, instead of updating only comparison result:

https://github.com/argoproj/argo-cd/blob/master/controller/controller.go#L117

Can you please update controller? Otherwise it will remove deployment info field

}

// Persist app deployment info
app, err = s.Update(ctx, app)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please set recent deployment only if actual deployment is successful?

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

LGTM, please merge after checks pass

@merenbach merenbach merged commit 98754c7 into argoproj:master Mar 16, 2018
@merenbach merenbach deleted the recent-deployments branch May 7, 2018 17:03
alexec pushed a commit that referenced this pull request Apr 24, 2019
leoluz pushed a commit to leoluz/argo-cd that referenced this pull request Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants