Skip to content
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

Jump to bottom

Update Provisioner example with ability to trigger dynamic provider change #513

Merged
merged 5 commits into from
May 5, 2020
Merged

Update Provisioner example with ability to trigger dynamic provider change #513

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3f68f5a
Add a user-settable changeToken property to ConnectionArgs that can b…
praneetloke Jan 14, 2020
83e29e1
Add new file
praneetloke Jan 14, 2020
d677300
Add the changeToken property to the input args of the provisioner and…
praneetloke Feb 3, 2020
b85f52b
Merge branch 'master' into praneetloke/update-provisioner
lukehoban Apr 29, 2020
d7df180
Fix lint error.
praneetloke May 5, 2020
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
Next Next commit
Add a user-settable changeToken property to ConnectionArgs that can b… 
…e used to detect changes to scripts.
  • Loading branch information
@praneetloke
praneetloke committed Jan 14, 2020
commit 3f68f5a248519dd846cad7a6e3f2bad694226114
6 changes: 4 additions & 2 deletions aws-ts-ec2-provisioners/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import * as aws from "@pulumi/aws";
import * as pulumi from "@pulumi/pulumi";
import * as provisioners from "./provisioners";
import { getFileHash } from "./util";

// Get the config ready to go.
const config = new pulumi.Config();
Expand Down Expand Up @@ -54,11 +55,12 @@ const server = new aws.ec2.Instance("server", {
keyName: keyName,
securityGroups: [ secgrp.name ],
});
const conn = {
const conn: provisioners.ConnectionArgs = {
host: server.publicIp,
username: "ec2-user",
privateKey,
privateKeyPassphrase,
privateKeyPassphrase,
changeToken: getFileHash("myapp.conf"),
Copy link
Member

Choose a reason for hiding this comment

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

Some indentation appears to be off here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah my editor was set to use Tabs for this repo for some reason. I have fixed it now.

};

// Copy a config file to our server.
Expand Down
7 changes: 6 additions & 1 deletion aws-ts-ec2-provisioners/provisioners/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ export interface ConnectionArgs {
username?: pulumi.Input<string>;
password?: pulumi.Input<string>;
privateKey?: pulumi.Input<string>;
privateKeyPassphrase?: pulumi.Input<string>;
privateKeyPassphrase?: pulumi.Input<string>;
/**
* A string value to control the replacement of the provisioner during each update.
* Provide a stable value if you do not need the provisioner to run each time you run an update.
*/
changeToken: string;
Copy link
Member

Choose a reason for hiding this comment

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

I feel like this doesn't quite belong on the ConnectionArgs - and might make more sense as a property on CopyFile args and RemoteExec args? In terraform, this is often called keepers - perhaps use that terminology (I've never found any terminology I particularly like for this).

Note that for the specific case of changing when the file contents change - it might be nice to actually have a first class hash property. Doing just that might be sufficient here?

Though looking at your original example - perhaps the reason you did it the way you did was to trigger both of these to re-run - which would not be possible with my suggestion above due to the lack of pulumi/pulumi#838.

Feel free to keep this as is - but perhaps consider keepers as the name?

As for doc comment here - the inconsistency is a little jarring - perhaps add comments on all properties? And either adopt the same // style, or update all existing doc comments to /** */ style?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like this doesn't quite belong on the ConnectionArgs - and might make more sense as a property on CopyFile args and RemoteExec args?

Yeah I agree -- it was a little hacky to put it with the ConnectionArgs, to be honest. I moved it to be a property of those component resources, as well as the provisioner dynamic resource's "state" input as well.

Though looking at your original example - perhaps the reason you did it the way you did was to trigger both of these to re-run

Yes, that's correct. The user can control which one they would like to re-run every time the file(s) content changes.

Feel free to keep this as is - but perhaps consider keepers as the name?

I don't like the name keepers, so I left the name changeToken, but can change it if you insist on using keepers.

}

// ConnectionType is the set of legal connection mechanisms to use. Default is SSH.
Expand Down