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

Add scenario check for LP 2017748 #972

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

dosaboy
Copy link
Member

@dosaboy dosaboy commented Sep 25, 2024

No description provided.

Copy link
Contributor

@zhhuabj zhhuabj left a comment

Choose a reason for hiding this comment

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

lgtm
I tested it, this is my test result - https://pastebin.canonical.com/p/KCmrsBC4JX/

Copy link
Member

@pponnuvel pponnuvel left a comment

Choose a reason for hiding this comment

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

LGTM

input:
path: 'var/log/neutron/neutron-ovn-metadata-agent.log'
search:
expr: '([\d-]+ [\d:]+.\d{3}) .+neutron.agent.ovn.metadata.agent \[-\] There is no metadata port for network \S+ or it has no MAC or IP addresses configured, tearing the namespace down if needed'
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to skip the "date" part? The rest of the strings would uniquely match in order to catch the issue. I know the log lines would have to have the timestamp for "constraints" to work but it's not necessary we regex it, right? Possibly a tiny improvement in compiling the regex is all.

This isn't unique to this scenario. But broadly applicable to most scenarios.

Copy link
Member Author

Choose a reason for hiding this comment

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

The short answer is no, this is necessary and the reason is because of the use here of "extra" constraints. See

def apply_extra_constraints(self, results):
for a more in-depth explanation.

@dosaboy dosaboy merged commit f525b07 into canonical:main Sep 26, 2024
7 checks passed
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.

4 participants