Skip to content
This repository has been archived by the owner on Jul 10, 2021. It is now read-only.

feat(reference/kubernetes): max-version-history docs #827

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

lwander
Copy link
Member

@lwander lwander commented Jun 4, 2018

No description provided.

@lwander
Copy link
Member Author

lwander commented Jun 4, 2018

@lwander lwander requested a review from dorbin June 4, 2018 18:44
Copy link
Contributor

@dorbin dorbin left a comment

Choose a reason for hiding this comment

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

Just a couple of questions, but lgtm


When set to a non-negative integer, this configures how many versions of a
resource to keep around. When more than `max-version-history` versions of a
Kubernetes artifact exist, Spinnaker deletes all older versions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't just delete the oldest each time? From 1 version to max number of them, then back to 1 each time?

Copy link
Member Author

Choose a reason for hiding this comment

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

Usually it does, but if you either go from not having this property set to setting it, or shrink the number, more than 1 will be deleted.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

a Deployment is managing, that is configured by
[`spec.revisionHistoryLimit`](https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#clean-up-policy).
If instead Spinnaker is deploying ReplicaSets directly without a Deployment,
this annotation does the job.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if there's a way to get this information at the top of the section, the use case for this property contrasted with the other property, before the "When set to a non-negative integer..." information.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this to clear up a potential confusion, I don't think this in particular should go at the top. In general I feel with the reference docs we can be a little more abstract. Maybe we should have a guide for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool by me. At first glance I don't see an obvious place in your existing k8sv2 guide for this. Feel free to follow up with me about what might be written there, but I'm ok with leaving this here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've assigned myself to a new guide to handle this: #829

I imagine it'll be pretty short & look similar to one of these: https://www.spinnaker.io/guides/user/kubernetes-v2/deploy-manifest/

@lwander lwander merged commit 724c0c6 into spinnaker:master Jun 4, 2018
@lwander lwander deleted the max-version-history branch June 4, 2018 20:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants