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.

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

openstack provider: ignore ec2 metadata if not present #1131

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rccrdpccl
Copy link

On baremetal platform, when reading metadata from a configdrive with openstack layout we get the following error:

# afterburn --provider openstack --attributes /etc/afterburn_metadata
Error: failed to run

Caused by:
    0: writing metadata attributes
    1: failed to open file '"/tmp/afterburn-7Uu4g0/ec2/latest/meta-data.json"'
    2: No such file or directory (os error 2)

This PR ignores ec2 metadata if not present:

# afterburn --provider openstack --attributes /etc/afterburn_metadata
# cat /etc/afterburn_metadata
AFTERBURN_OPENSTACK_INSTANCE_UUID=30aed4c7-05a4-4d15-be9b-9194dc1b5ad1

@rccrdpccl
Copy link
Author

/cc @jlebon


let metadata_ec2 = match self.read_metadata_ec2() {
Ok(metadata) => metadata,
Err(error) => return Ok(out),
Copy link
Member

Choose a reason for hiding this comment

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

First, let's instead change read_metadata_ec2() to return a Result<Option<MetadataEc2JSON>>. So then it would return Ok(None) in this case. It should only return that if the error was ENOENT. Grep the codebase for NotFound for examples of this.

Second, this is a lot of metadata we're not injecting now. Are you sure that information is not available in the openstack/meta_data.json? I was under the impression when we talked about this that it was.

Copy link
Author

Choose a reason for hiding this comment

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

All the metadata we are not injecting seems to be either ec2 related (INSTANCE_ID, INSTANCE_TYPE), or somewhat irrelevant, at least in my use case: IPV4_LOCAL and IPV4_PUBLIC .

The information provided by metal3 is the following:

{
  "local-hostname": "bmh-vm-02",
  "local_hostname": "bmh-vm-02",
  "metal3-name": "bmh-vm-02",
  "metal3-namespace": "test-capi",
  "name": "bmh-vm-02",
  "uuid": "30aed4c7-05a4-4d15-be9b-9194dc1b5ad1"
}

We can see uuid, hostname - although none of its label it's matching the current provider's key - and other metal3 related keys (useful to build the providerID pattern expected by metal3).

@rccrdpccl rccrdpccl force-pushed the ignore-ec2-if-not-present branch from 83ffc74 to ebf930c Compare November 15, 2024 11:21
let file = match File::open(&filename) {
Ok(file) => file,
Err(e) if e.kind() == NotFound => return Ok(None),
Err(e) => return Err(anyhow::Error::new(e).context(format!("failed to open file '{filename:?}'"))),
Copy link
Member

Choose a reason for hiding this comment

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

with_context

Comment on lines 114 to 118
match Self::parse_metadata_ec2(bufrd)
.with_context(|| format!("failed to parse file '{filename:?}'")) {
Ok(metadata) => Ok(Some(metadata)),
Err(e) => Err(e),
}
Copy link
Member

Choose a reason for hiding this comment

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

Instead of the match, you can .map(Some) to wrap it in an Option.

Comment on lines 162 to 163
let metadata_ec2 = match self.read_metadata_ec2() {
Ok(Some(metadata)) => metadata,
Ok(None) => return Ok(out),
Err(e) => return Err(e),
};
Copy link
Member

Choose a reason for hiding this comment

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

You can do something like:

let Some(...) = self...()? else {
    return Ok(out);
};

@@ -10,6 +10,8 @@ Major changes:

Minor changes:

- Openstack: do not fail if ec2 metadata is not found
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Openstack: do not fail if ec2 metadata is not found
- OpenStack: do not fail if ec2 metadata is not found

@rccrdpccl rccrdpccl force-pushed the ignore-ec2-if-not-present branch from ebf930c to 5158090 Compare November 18, 2024 09:37
@rccrdpccl rccrdpccl requested a review from jlebon November 19, 2024 09:10
@rccrdpccl rccrdpccl force-pushed the ignore-ec2-if-not-present branch 2 times, most recently from 3adc3a1 to dae8976 Compare December 9, 2024 11:39
Signed-off-by: Riccardo Piccoli <rpiccoli@redhat.com>
@rccrdpccl rccrdpccl force-pushed the ignore-ec2-if-not-present branch from dae8976 to f1a9378 Compare December 16, 2024 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants