Skip to content

add Tags Name for Master and Worker Nodes in the Cloudformation UPI AWS#1996

Closed
rcarrata wants to merge 1 commit intoopenshift:masterfrom
rcarrata:master
Closed

add Tags Name for Master and Worker Nodes in the Cloudformation UPI AWS#1996
rcarrata wants to merge 1 commit intoopenshift:masterfrom
rcarrata:master

Conversation

@rcarrata
Copy link
Copy Markdown

What is this PR for?

When a OCP 4.x is deployed in UPI mode using the AWS Cloudformations templates, there is not the proper Tag Names for the Workers and Masters Nodes deployed (ec2 instances tag names).
Furthermore, there isn't tagged the security groups.

For this reason is complicated to identify which one are the masters or the workers nodes deployed (and also who is the master1, 2 or 3). Same thing for the security groups of master/worker nodes.

What is this PR About?

This PR add the tag name for the worker / master nodes (ec2 instances vm), giving a describing name for each one of them. Furthermore, also define the tag names for the security groups attached to master/worker nodes.

How is this PR tested?

The code pull requested is tested in OCP upi 4.1.3/4.1.4 and worked perfectly. The ec2 instances for masters/workers were identified easily, as same as the security groups attached to them.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 16, 2019
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

Hi @rcarrata. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 16, 2019
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rcarrata
To complete the pull request process, please assign abhinavdahiya
You can assign the PR to them by writing /assign @abhinavdahiya in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@rcarrata
Copy link
Copy Markdown
Author

/assign @abhinavdahiya

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We usually have them indexed from 0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@abhinavdahiya sure! I changed the code, and now are indexed from 0.

Can you please review the changes?

@abhinavdahiya
Copy link
Copy Markdown
Contributor

/ok-to-test

/test e2e-aws-upi

@openshift-ci-robot openshift-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 22, 2019
@rcarrata
Copy link
Copy Markdown
Author

rcarrata commented Jul 22, 2019

@abhinavdahiya could you please check the 2 CI jobs that failed? Is due to the modificacions of the tagName into the Cloudformation Templates? Thanks in advance!

@abhinavdahiya
Copy link
Copy Markdown
Contributor

/test e2e-aws-upi

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@rcarrata: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 6181e28 link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-aws-upi 6181e28 link /test e2e-aws-upi

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.

Details

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.

@abhinavdahiya
Copy link
Copy Markdown
Contributor

Can you please link the bugzilla for this change. With openshift working towards 4.2 only approved features are allowed to merge to installer.

@abhinavdahiya
Copy link
Copy Markdown
Contributor

ping @rcarrata

Can you please link the bugzilla for this change. With openshift working towards 4.2 only approved features are allowed to merge to installer.

@wking
Copy link
Copy Markdown
Member

wking commented Jul 30, 2019

We don't need tags on these machines, do we? For example, see 9572a06 (#1649), removing a number of things from the templates that we don't actually need for a successful UPI flow. The templates are not intended to provide a comfortable working environment, they're intended to show the minimal requirements for a functioning cluster. Obviously feel free to add usability stuff like this locally when launching your cluster, but I don't think we want unnecessary tags and such in the official templates.

@wking
Copy link
Copy Markdown
Member

wking commented Jul 30, 2019

Also note that you can do things like:

$ aws cloudformtation create-stack --tags Key=Name,Value=your-infra-name-worker-0 ...

to create a compute machine from the 06_cluster_worker_node.yaml template with your preferred tag.

@abhinavdahiya
Copy link
Copy Markdown
Contributor

Also note that you can do things like:

$ aws cloudformtation create-stack --tags Key=Name,Value=your-infra-name-worker-0 ...

to create a compute machine from the 06_cluster_worker_node.yaml template with your preferred tag.

/close

I think the information provided by @wking should be a valid solution.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@abhinavdahiya: Closed this PR.

Details

In response to this:

Also note that you can do things like:

$ aws cloudformtation create-stack --tags Key=Name,Value=your-infra-name-worker-0 ...

to create a compute machine from the 06_cluster_worker_node.yaml template with your preferred tag.

/close

I think the information provided by @wking should be a valid solution.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants