From 27edcf607fff481f6c494c02f8994a06018b2b1f Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Fri, 1 Jun 2018 10:51:22 +0100 Subject: [PATCH 1/4] Add dry-run proposal --- docs/proposals/puppet-wing-dry-run.rst | 140 +++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 docs/proposals/puppet-wing-dry-run.rst diff --git a/docs/proposals/puppet-wing-dry-run.rst b/docs/proposals/puppet-wing-dry-run.rst new file mode 100644 index 0000000000..99458fed91 --- /dev/null +++ b/docs/proposals/puppet-wing-dry-run.rst @@ -0,0 +1,140 @@ +.. vim:set ft=rst spell: + +####################### +Puppet Dry Run Via Wing +####################### + +Background +========== + +Using puppet's noop mode we can perform a proper dry-run of the changes that be +performed on cluster instances. + +See http://nrvale0.github.io/posts/the-basics-of-puppet-noop/ for an overview on noop mode. + +Glossary +-------- + +- **Puppet dry-run**: a puppet mode which calculates changes, but does not actually apply them to the instance. + +Problem +------- + +Currently there is no way to verify what a ``puppet apply`` will do to nodes. +In some situations, we would like to review the changes that Puppet would do on +the system without actually applying them, to prevent causing destructive +configuration changes. + +We need a way to represent applying a set of puppet manifests to an instance, +and a way to view the output of an application. + +See: + +- https://github.com/jetstack/tarmak/issues/224 + +Objective +========= + +Verify puppet will make sensible and expected changes to the cluster when running:: + + $ tarmak cluster puppet-plan + +which complements ``cluster puppet-plan`` verifying Terraform changes. + +Changes +======= + +To implement this, new objects will be added to Wing's API. + +New API objects +--------------- + +``PuppetManifest`` +****************** + +A resource representing a set of puppet manifests to apply to an instance. + +This bundles together the source (S3, google Cloud Storage, etc) and +verification of the source (a hash, PGP signature, etc). + +.. code-block:: yaml + + kind: PuppetManifest + metadata: + name: example-manifest + hash: sha256:34242343 + source: + s3: + bucketName: something + path: something-else/puppet.tar.gz + +The source will be structured similarly to kubernetes' ``VolumeSource``, with a +field for each type of source. For example, something like this in ``types.go``: + +.. code-block:: go + + type ManifestSource struct { + S3 *S3ManifestSource `json:"s3ManifestSource"` + } + + type S3ManifestSource struct { + BucketName string `json:"bucketName"` + } + + +``PuppetJob`` +************* + +A resource representing the application of a ``PuppetManifest`` on an instance: + +.. code-block:: yaml + + kind: PuppetJob + metadata: + name: example-job + spec: + manifestRef: + name: example-manifest + operation: "dry-run" + instanceID: 1234 + status: + exitCode: 1 + messages: "" + +This references a pre-existing ``PuppetManifest``, and performs the specified +action on an instance. + +Changes to existing API objects +------------------------------- + +``InstanceSpec`` will have a ``manifestRef`` field also linking to a ``PuppetManifest`` resource. +This will be the manifest applied to the instance. + +Changes to tarmak CLI +--------------------- + +The tarmak CLI needs modification to add support for creating +``PuppetManifest`` and ``PuppetJob`` resources. + +The planned workflow is to run:: + + $ tarmak cluster puppet-plan + +which creates ``PuppetJob`` resources for either a subset of instances of each +type in the current cluster, or all instances. This blocks until +``PuppetJob.Status.ExitCode`` for each created job is populated. + +It would be nice to filter and only display results based on the exit code of puppet, but it seems the exit code is always ``0`` when ``--noop`` is enabled:: + + https://tickets.puppetlabs.com/browse/PUP-686 + +Notable items +============= + +Concerns +-------- + +- Performing updates to puppet manifests will leave ``PuppetJob`` and + ``PuppetManifest`` resources hanging around. Should there be an automated clean + up process for stale items? +- We need to think about how to handle ``PuppetJob`` resources timing out in the case of an instance failure during a plan. From e797aca16919ba0a251c8ef634b0aeb8a846e14b Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Fri, 1 Jun 2018 17:10:11 +0100 Subject: [PATCH 2/4] Add words --- docs/spelling_wordlist.txt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/spelling_wordlist.txt b/docs/spelling_wordlist.txt index 7a56c153d9..b0b5451b57 100644 --- a/docs/spelling_wordlist.txt +++ b/docs/spelling_wordlist.txt @@ -65,3 +65,12 @@ Jenkins prepended username loopback +noop +admin +offline +plugin +google +checklist +ubuntu +workflow +prometheus From 51c618d5fb3e08a8712b321e74788c44415e87cc Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Fri, 1 Jun 2018 17:33:46 +0100 Subject: [PATCH 3/4] PuppetJob -> WingJob --- docs/proposals/puppet-wing-dry-run.rst | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/docs/proposals/puppet-wing-dry-run.rst b/docs/proposals/puppet-wing-dry-run.rst index 99458fed91..f9140f69ec 100644 --- a/docs/proposals/puppet-wing-dry-run.rst +++ b/docs/proposals/puppet-wing-dry-run.rst @@ -39,7 +39,7 @@ Verify puppet will make sensible and expected changes to the cluster when runnin $ tarmak cluster puppet-plan -which complements ``cluster puppet-plan`` verifying Terraform changes. +which complements ``cluster plan`` verifying Terraform changes. Changes ======= @@ -82,18 +82,18 @@ field for each type of source. For example, something like this in ``types.go``: } -``PuppetJob`` +``WingJob`` ************* A resource representing the application of a ``PuppetManifest`` on an instance: .. code-block:: yaml - kind: PuppetJob + kind: WingJob metadata: name: example-job spec: - manifestRef: + puppetManifestRef: name: example-manifest operation: "dry-run" instanceID: 1234 @@ -104,10 +104,17 @@ A resource representing the application of a ``PuppetManifest`` on an instance: This references a pre-existing ``PuppetManifest``, and performs the specified action on an instance. +If, in the future, we support other configuration management tools (ansible, +chef, etc), these would be represented by separate fields. + +Performing updates to puppet manifests will leave ``WingJob`` and +``PuppetManifest`` resources hanging around. To prevent this, wing should only +keep the last 15 (or some other number) WingJobs for each instance. + Changes to existing API objects ------------------------------- -``InstanceSpec`` will have a ``manifestRef`` field also linking to a ``PuppetManifest`` resource. +``InstanceSpec`` will have a ``puppetManifestRef`` field also linking to a ``PuppetManifest`` resource. This will be the manifest applied to the instance. Changes to tarmak CLI @@ -120,9 +127,9 @@ The planned workflow is to run:: $ tarmak cluster puppet-plan -which creates ``PuppetJob`` resources for either a subset of instances of each +which creates ``WingJob`` resources for either a subset of instances of each type in the current cluster, or all instances. This blocks until -``PuppetJob.Status.ExitCode`` for each created job is populated. +``WingJob.Status.ExitCode`` for each created job is populated. It would be nice to filter and only display results based on the exit code of puppet, but it seems the exit code is always ``0`` when ``--noop`` is enabled:: @@ -134,7 +141,4 @@ Notable items Concerns -------- -- Performing updates to puppet manifests will leave ``PuppetJob`` and - ``PuppetManifest`` resources hanging around. Should there be an automated clean - up process for stale items? -- We need to think about how to handle ``PuppetJob`` resources timing out in the case of an instance failure during a plan. +- We need to think about how to handle ``WingJob`` resources timing out in the case of an instance failure during a plan. From c1f349e79c8b71f4be792b2195fe37fde51d5528 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 12 Jun 2018 17:57:53 +0100 Subject: [PATCH 4/4] Updates --- docs/proposals/puppet-wing-dry-run.rst | 38 ++++++++++++++++---------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/docs/proposals/puppet-wing-dry-run.rst b/docs/proposals/puppet-wing-dry-run.rst index f9140f69ec..e11563e12f 100644 --- a/docs/proposals/puppet-wing-dry-run.rst +++ b/docs/proposals/puppet-wing-dry-run.rst @@ -37,7 +37,7 @@ Objective Verify puppet will make sensible and expected changes to the cluster when running:: - $ tarmak cluster puppet-plan + $ tarmak cluster config-plan which complements ``cluster plan`` verifying Terraform changes. @@ -49,17 +49,18 @@ To implement this, new objects will be added to Wing's API. New API objects --------------- -``PuppetManifest`` -****************** +``PuppetTarget`` +**************** -A resource representing a set of puppet manifests to apply to an instance. +A resource representing a set of puppet modules and additional information +(hieradata etc) to apply to an instance. This bundles together the source (S3, google Cloud Storage, etc) and verification of the source (a hash, PGP signature, etc). .. code-block:: yaml - kind: PuppetManifest + kind: PuppetTarget metadata: name: example-manifest hash: sha256:34242343 @@ -74,7 +75,7 @@ field for each type of source. For example, something like this in ``types.go``: .. code-block:: go type ManifestSource struct { - S3 *S3ManifestSource `json:"s3ManifestSource"` + S3 *S3ManifestSource `json:"s3"` } type S3ManifestSource struct { @@ -85,7 +86,7 @@ field for each type of source. For example, something like this in ``types.go``: ``WingJob`` ************* -A resource representing the application of a ``PuppetManifest`` on an instance: +A resource representing the application of a ``PuppetTarget`` on an instance: .. code-block:: yaml @@ -93,7 +94,7 @@ A resource representing the application of a ``PuppetManifest`` on an instance: metadata: name: example-job spec: - puppetManifestRef: + puppetTargetRef: name: example-manifest operation: "dry-run" instanceID: 1234 @@ -101,31 +102,40 @@ A resource representing the application of a ``PuppetManifest`` on an instance: exitCode: 1 messages: "" -This references a pre-existing ``PuppetManifest``, and performs the specified +This references a pre-existing ``PuppetTarget``, and performs the specified action on an instance. If, in the future, we support other configuration management tools (ansible, chef, etc), these would be represented by separate fields. Performing updates to puppet manifests will leave ``WingJob`` and -``PuppetManifest`` resources hanging around. To prevent this, wing should only +``PuppetTarget`` resources hanging around. To prevent this, wing should only keep the last 15 (or some other number) WingJobs for each instance. Changes to existing API objects ------------------------------- -``InstanceSpec`` will have a ``puppetManifestRef`` field also linking to a ``PuppetManifest`` resource. -This will be the manifest applied to the instance. +``InstanceSpec`` will have a ``spec.puppetTargetRef`` field also linking to a +``PuppetTarget`` resource. This will be the manifest applied to the instance. +A corresponding ``status.puppetTargetRef`` will also be added to record the +latest known applied ``PuppetTarget``. + +When the ``spec.puppetTargetRef`` field is updated on an instance, a new +``WingJob`` is created with ``operation`` set to ``apply``. + +If a ``WingJob`` applies a different ``PuppetTarget``, the +``spec.puppetTargetRef`` will not be updated, however +``status.puppetTargetRef`` will be. Changes to tarmak CLI --------------------- The tarmak CLI needs modification to add support for creating -``PuppetManifest`` and ``PuppetJob`` resources. +``PuppetTarget`` and ``PuppetJob`` resources. The planned workflow is to run:: - $ tarmak cluster puppet-plan + $ tarmak cluster config-plan which creates ``WingJob`` resources for either a subset of instances of each type in the current cluster, or all instances. This blocks until