Skip to content

bug 1715118 updating parameter value#15059

Merged
kalexand-rh merged 1 commit intoopenshift:enterprise-4.1from
kalexand-rh:BZ1715118
May 30, 2019
Merged

bug 1715118 updating parameter value#15059
kalexand-rh merged 1 commit intoopenshift:enterprise-4.1from
kalexand-rh:BZ1715118

Conversation

@kalexand-rh
Copy link
Copy Markdown
Contributor

@kalexand-rh kalexand-rh added this to the Future Release milestone May 29, 2019
@kalexand-rh kalexand-rh self-assigned this May 29, 2019
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 29, 2019
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

zone needs to be changed to region

I don't know what <dns_stack> is. There's no bit in the documentation describing what should go in this position. I can guess from what actually gets deployed is it's --RegisterNlbIpTargets-
This makes the example look obnoxious ...
"ParameterValue": "arn:aws:lambda:<region>:<account_number>:function:<infrastructure name>-<stack name>-RegisterNlbIpTargets-<random hash>"
It's verbose but clearer than before. The excessive length is because the resource names are too long. and doesn't need to be in the resource name. The random hash is going to prevent name conflict. They're all tagged correctly so well searchable.

AWS uses the following generic examples for Lamda ...

arn:aws:lambda:region:account-id:function:function-name
arn:aws:lambda:region:account-id:function:function-name:alias-name
arn:aws:lambda:region:account-id:function:function-name:version
arn:aws:lambda:region:account-id:event-source-mappings:event-source-mapping-id

This is taken from here. Then web browser search for AWS Lambda

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mazzystr, I'm making some updates based on you feedback. Thanks! Will you PTAL?

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 29, 2019
@mazzystr
Copy link
Copy Markdown

LGTM! Thank you!

@kalexand-rh
Copy link
Copy Markdown
Contributor Author

Thanks Chris! Merging.

@kalexand-rh kalexand-rh merged commit e10d9de into openshift:enterprise-4.1 May 30, 2019
@kalexand-rh kalexand-rh deleted the BZ1715118 branch May 30, 2019 15:37
},
{
"ParameterKey": "WorkerSubnet", <5>
"ParameterValue": "yes" <6>
Copy link
Copy Markdown

@mazzystr mazzystr May 30, 2019

Choose a reason for hiding this comment

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

Sorry ... found another problem. This parameter key should be called PrivateSubnet. The Value should be sg- string. The parameter should be modified in the template yaml also @wking

Since there is only one subnet parameter for workers nodes how do I span subnets across availability zones?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

kalexand-rh added a commit to kalexand-rh/installer that referenced this pull request May 31, 2019
Because we can no longer use "UPI" in customer-facing documentation and the CloudFormation templates in /installer are published in /openshift-docs, I want to change the source templates instead of having to always reproduce the fix in the docs repo.

I'm making a few other style and changes to the parameter descriptions.

Per a comment on a docs PR (openshift/openshift-docs#15059), the subnet type for the worker CloudFormation template  needs to change to PrivateSubnet from WorkerSubnet. Trevor clarified that the subnet in the worker does not need to be private. s/PrivateSubnet/Subnet in just that CloudFormation Template.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.1 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants