Add dry-run proposal #273
Add dry-run proposal #273
Conversation
jetstack-bot
added
the
release-note-none
jetstack-bot
added
the
size/L
| Changes to existing API objects | ||
| ------------------------------- | ||
|
|
||
| ``InstanceSpec`` will have a ``manifestRef`` field also linking to a ``PuppetManifest`` resource. |
There was a problem hiding this comment.
This should be a puppetManifestRef
|
|
||
| - Performing updates to puppet manifests will leave ``PuppetJob`` and | ||
| ``PuppetManifest`` resources hanging around. Should there be an automated clean | ||
| up process for stale items? |
There was a problem hiding this comment.
I think similar to how jobs clear a pod associated with them we should only keep like the last 15 WingJobs. (Ideally that is configurable by a variable in the Instance type)
|
|
||
| $ tarmak cluster puppet-plan | ||
|
|
||
| which creates ``PuppetJob`` resources for either a subset of instances of each |
There was a problem hiding this comment.
I think it should be called more generic WingJob
jetstack-bot
added
the
needs-rebase
kragniz
force-pushed
the
puppet-dry-run-proposal
branch
from
9687992 to
bb266b9
Compare
jetstack-bot
removed
the
needs-rebase
|
|
||
| Verify puppet will make sensible and expected changes to the cluster when running:: | ||
|
|
||
| $ tarmak cluster puppet-plan |
There was a problem hiding this comment.
To prepare for the (slim) chance that we have multiple options for instance configuration, we shouldn't tie this command to puppet - instead something like tarmak cluster config-plan?
| New API objects | ||
| --------------- | ||
|
|
||
| ``PuppetManifest`` |
There was a problem hiding this comment.
As PuppetManifest represents multiple manifests (and lots of other stuff), it might be confusing having the singular in the name. Perhaps PuppetTarget would be more appropriate, which could later be expanded to AnsibleTarget.
| ``PuppetManifest`` | ||
| ****************** | ||
|
|
||
| A resource representing a set of puppet manifests to apply to an instance. |
There was a problem hiding this comment.
Worth mentioning that the configuration referenced by a PuppetManifest resource contains lots of other stuff (e.g. puppet modules, hieradata etc.)
| .. code-block:: go | ||
|
|
||
| type ManifestSource struct { | ||
| S3 *S3ManifestSource `json:"s3ManifestSource"` |
There was a problem hiding this comment.
json:"s3ManifestSource" -> json:"s3" to match with the configuration block above?
| Changes to existing API objects | ||
| ------------------------------- | ||
|
|
||
| ``InstanceSpec`` will have a ``puppetManifestRef`` field also linking to a ``PuppetManifest`` resource. |
There was a problem hiding this comment.
Worth clarifying how this field would interact with the new resources described above - what would happen if the puppetManifestRef field on InstanceSpec changed, would wing work towards trying to apply the new puppet manifest without creating a corresponding WingJob resource? And if a new WingJob resource was created with an operation of apply rather than dry-run would this field be updated if the configuration was applied successfully?
|
/assign @dippynark |
kragniz
force-pushed
the
puppet-dry-run-proposal
branch
from
bb266b9 to
c1f349e
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jetstack-bot
added
the
needs-rebase
|
@kragniz: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/unassign |
|
Would be great to get that updated an merged @kragniz |
|
@kragniz: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: