Skip to content

Conversation

@cjschaef
Copy link
Member

Add the IP addresses for IBM bootstrap and master nodes to allow
collecting of logs from those nodes.

@openshift-ci openshift-ci bot requested review from jstuever and kirankt January 15, 2022 00:21
Copy link
Contributor

@jstuever jstuever left a comment

Choose a reason for hiding this comment

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

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 17, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstuever

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

The pull request process is described 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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 17, 2022
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be named control_plane_ips to be picked up by default by the bootstrap gather.

Suggested change
output "ip_addresses" {
output "control_plane_ips" {

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, will update

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend that you use the full splat operator rather than the legacy one.

Suggested change
value = ibm_is_instance.master_node.*.primary_network_interface.0.primary_ipv4_address
value = ibm_is_instance.master_node[*].primary_network_interface[0].primary_ipv4_address

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, will update

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Add the IP addresses for IBM bootstrap and master nodes to allow
collecting of logs from those nodes.
@cjschaef cjschaef force-pushed the ibmcloud-output-ips branch from 2a2b60b to 0561351 Compare January 18, 2022 14:56
@staebler
Copy link
Contributor

/lgtm
/hold for results from e2e-ibmcloud

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 18, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 18, 2022

@cjschaef: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-metal-ipi-ovn-ipv6 0561351 link false /test e2e-metal-ipi-ovn-ipv6
ci/prow/okd-e2e-aws 0561351 link false /test okd-e2e-aws
ci/prow/e2e-ovirt 0561351 link false /test e2e-ovirt
ci/prow/e2e-aws-workers-rhel7 0561351 link false /test e2e-aws-workers-rhel7
ci/prow/e2e-ibmcloud 0561351 link false /test e2e-ibmcloud
ci/prow/e2e-aws-workers-rhel8 0561351 link false /test e2e-aws-workers-rhel8
ci/prow/e2e-crc 0561351 link false /test e2e-crc
ci/prow/e2e-aws-single-node 0561351 link false /test e2e-aws-single-node
ci/prow/okd-e2e-aws-upgrade 0561351 link false /test okd-e2e-aws-upgrade

Full PR test history. Your PR dashboard.

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.

@cjschaef
Copy link
Member Author

Tested a gather bootstrap during a test run, with a locally build version of this code, appears the logs were gathered as expected

# bin/openshift-install gather bootstrap --dir cluster-deploys/ipi-dev-test-90
INFO Pulling debug logs from the bootstrap machine 
INFO Bootstrap gather logs captured here "/go/src/github.com/openshift/installer/cluster-deploys/ipi-dev-test-90/log-bundle-20220121204118.tar.gz" 

# cat cluster-deploys/ipi-dev-test-90/bootstrap.tfvars.json 
{"bootstrap_ip":"150.239.113.232"}root@3065a3db5912:/go/src/github.com/openshift/installer# 

# cat cluster-deploys/ipi-dev-test-90/master.tfvars.json    
{"control_plane_ips":["10.241.0.5","10.241.64.6","10.241.128.6"]}

@staebler
Copy link
Contributor

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2022
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@cjschaef cjschaef changed the title Add IP outputs for IBM terraform instances Bug 2043731: Add IP outputs for IBM terraform instances Jan 21, 2022
@openshift-ci openshift-ci bot added bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Jan 21, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2022

@cjschaef: This pull request references Bugzilla bug 2043731, which is invalid:

  • expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 2043731: Add IP outputs for IBM terraform instances

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.

@kirankt
Copy link
Contributor

kirankt commented Jan 21, 2022

/bugzilla refresh

@openshift-ci openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jan 21, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 21, 2022

@kirankt: This pull request references Bugzilla bug 2043731, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.10.0) matches configured target release for branch (4.10.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

Details

In response to this:

/bugzilla refresh

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 openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Jan 21, 2022
@openshift-bot
Copy link
Contributor

/retest-required

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 716cf76 into openshift:master Jan 22, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 22, 2022

@cjschaef: All pull requests linked via external trackers have merged:

Bugzilla bug 2043731 has been moved to the MODIFIED state.

Details

In response to this:

Bug 2043731: Add IP outputs for IBM terraform instances

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.

@pamoedom
Copy link
Contributor

pamoedom commented Jan 24, 2022

Hi @cjschaef, FYI, please note that we are seeing the following related warning messages in our build logs:

01-24 10:21:40.903  level=debug msg=Warning: Value for undeclared variable
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug msg=The root module does not declare a variable named "control_plane_ips" but a
01-24 10:21:40.903  level=debug msg=value was found in file
01-24 10:21:40.903  level=debug msg="/tmp/openshift-install-bootstrap-869967759/master.tfvars.json". To use this
01-24 10:21:40.903  level=debug msg=value, add a "variable" block to the configuration.
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug msg=Using a variables file to set an undeclared variable is deprecated and will
01-24 10:21:40.903  level=debug msg=become an error in a future release. If you wish to provide certain "global"
01-24 10:21:40.903  level=debug msg=settings to all configurations in your organization, use TF_VAR_...
01-24 10:21:40.903  level=debug msg=environment variables to set these instead.
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug msg=Warning: Value for undeclared variable
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug msg=The root module does not declare a variable named "bootstrap_ip" but a value
01-24 10:21:40.903  level=debug msg=was found in file
01-24 10:21:40.903  level=debug msg="/tmp/openshift-install-bootstrap-869967759/bootstrap.tfvars.json". To use
01-24 10:21:40.903  level=debug msg=this value, add a "variable" block to the configuration.
01-24 10:21:40.903  level=debug
01-24 10:21:40.903  level=debug msg=Using a variables file to set an undeclared variable is deprecated and will
01-24 10:21:40.903  level=debug msg=become an error in a future release. If you wish to provide certain "global"
01-24 10:21:40.903  level=debug msg=settings to all configurations in your organization, use TF_VAR_...
01-24 10:21:40.903  level=debug msg=environment variables to set these instead.

AFAIK, those new variables shouldn't be also be declared within data/data/ibmcloud/variables-ibmcloud.tf data/data/ibmcloud/master/variables.tf & data/data/ibmcloud/bootstrap/variables.tf for consistency?

Best Regards.

@cjschaef
Copy link
Member Author

@pamoedom I'm not sure TBH, as I thought I ran into errors when trying to declare them, or perhaps I need to declare them a certain way for outputs. Let me retry testing that here and get back.

@cjschaef cjschaef deleted the ibmcloud-output-ips branch January 24, 2022 14:20
@cjschaef
Copy link
Member Author

@pamoedom I'll double check terraform formatting on the warning above (Value for undeclared variable), as adding them to outputs.tf initially causes failures during openshift-installer create cluster, since it expects an input value for those variables in variables.tf

time="2022-01-24T16:02:37Z" level=debug msg="Initializing the backend..."
time="2022-01-24T16:02:37Z" level=debug
time="2022-01-24T16:02:37Z" level=debug msg="Initializing provider plugins..."
time="2022-01-24T16:02:37Z" level=debug msg="- Finding latest version of openshift/local/ibm..."
time="2022-01-24T16:02:37Z" level=debug msg="- Installing openshift/local/ibm v1.0.0..."
time="2022-01-24T16:02:39Z" level=debug msg="- Installed openshift/local/ibm v1.0.0 (unauthenticated)"
time="2022-01-24T16:02:39Z" level=debug
time="2022-01-24T16:02:39Z" level=debug msg="Terraform has created a lock file .terraform.lock.hcl to record the provider"
time="2022-01-24T16:02:39Z" level=debug msg="selections it made above. Include this file in your version control repository"
time="2022-01-24T16:02:39Z" level=debug msg="so that Terraform can guarantee to make the same selections by default when"
time="2022-01-24T16:02:39Z" level=debug msg="you run \"terraform init\" in the future."
time="2022-01-24T16:02:39Z" level=debug
time="2022-01-24T16:02:39Z" level=debug msg="Terraform has been successfully initialized!"
time="2022-01-24T16:02:39Z" level=debug
time="2022-01-24T16:02:39Z" level=debug msg="You may now begin working with Terraform. Try running \"terraform plan\" to see"
time="2022-01-24T16:02:39Z" level=debug msg="any changes that are required for your infrastructure. All Terraform commands"
time="2022-01-24T16:02:39Z" level=debug msg="should now work."
time="2022-01-24T16:02:39Z" level=debug
time="2022-01-24T16:02:39Z" level=debug msg="If you ever set or change modules or backend configuration for Terraform,"
time="2022-01-24T16:02:39Z" level=debug msg="rerun this command to reinitialize your working directory. If you forget, other"
time="2022-01-24T16:02:39Z" level=debug msg="commands will detect it and remind you to do so if necessary."
time="2022-01-24T16:02:41Z" level=error
time="2022-01-24T16:02:41Z" level=error msg="Error: No value for required variable"
time="2022-01-24T16:02:41Z" level=error
time="2022-01-24T16:02:41Z" level=error msg="  on ../../../../../tmp/openshift-install-bootstrap-3542695119/variables.tf line 58:"
time="2022-01-24T16:02:41Z" level=error msg="  58: variable \"bootstrap_ip\" {"
time="2022-01-24T16:02:41Z" level=error
time="2022-01-24T16:02:41Z" level=error msg="The root module input variable \"bootstrap_ip\" is not set, and has no default"
time="2022-01-24T16:02:41Z" level=error msg="value. Use a -var or -var-file command line argument to provide a value for"
time="2022-01-24T16:02:41Z" level=error msg="this variable."
time="2022-01-24T16:02:41Z" level=error
time="2022-01-24T16:02:41Z" level=error msg="Failed to read tfstate: open /tmp/openshift-install-bootstrap-3542695119/terraform.bootstrap.tfstate: no such file or directory"
time="2022-01-24T16:02:41Z" level=fatal msg="failed to fetch Cluster: failed to generate asset \"Cluster\": failed to create cluster: failed to apply Terraform: failed to complete the change"

Also, I don't see the other platforms specify the bootstrap_ip or master variable (different platforms do different things)
https://github.com/openshift/installer/search?p=1&q=bootstrap_ip

But I'm not knowledgeable enough with Terraform to know whether this warning is an issue or not yet.

@staebler
Copy link
Contributor

I would not bother spending too much time on the errors as they are inconsequential at the moment. The installer pins the terraform version, so there is no concern that it will start to fail for a given OpenShift version due to terraform no longer supporting supplying a value for an undeclared variable. Many of the other platforms have similar warnings. We will likely clean this all up in 4.11.

With that said, if you want to resolve the warnings, you would need to declare "bootstrap_ip" in data/data/ibmcloud/master/variables.tf and both "bootstrap_ip" and "control_plane_ips" in data/data/ibmcloud/bootstrap/variables.tf. For the latter file, the variables would need to have a default declared since the values will only be preset during the destroy phase.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-medium Referenced Bugzilla bug's severity is medium for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants