fix(cloudstack): Improve domain-name DHCP lease lookup (Cloudstack) #6554
Conversation
* Adding case-insensitive options for systemd-networkd leases ("DOMAINNAME", "Domain", "domain-name").
* Falling back gracefully from systemd leases to ISC dhclient leases.
* Including dhcpcd ephemeral leases as an additional fallback.
* Returning an empty string when no domain name found instead of None for non-fatal missing cases.
|
@TheRealFalcon @holmanb Not sure why the CLA check is failing. I'm pretty sure I signed a CLA a long time ago. Here is another PR that I've commited and was merged in cloud-init : #4660 |
|
@CodeBleu You'll need to sign the new CLA, sorry. |
|
@holmanb the CLA was signed by my company. I closed and re-opend the PR, but it appears it's still not validating my CLA check. Not sure if we should get an "approved" email after submitting the CLA or not? |
Thanks for signing. I think the email address associated with your github account would need to match the company domain. It looks like the check failed with a github.com domain -> |
The whole reason to have that email in github is to not actually show my email! 😄 The email domain that is used to sign the CLA is a valid email I have under my github account. There should be a way to have the CLA still validate it with being able to use the obscure email that Github provides. |
blackboxsw
left a comment
blackboxsw
left a comment
There was a problem hiding this comment.
Thank you @CodeBleu for continuing to contribute to cloud-init and making it better on more platforms.
I've pinged internally the right persons to attempt to sort what is missing in CLA validation. In the meantime, here's a review which hopefully gets us to landing closer to landing this branch. We'd definitely like to see some unit test coverage of Domain and domain-name values as well as NoDHCPLeaseError condition being raised.
Other than that. this looks good to me.
* Addition of parameterized unittests for "DOMAINNAME", "Domain", and "domain-name" lease keys in _get_domainname.
* Handling of NoDHCPLeaseError condition with tests.
* Restriction of exception handling to known exception types (NoDHCPLeaseError, FileNotFoundError, AttributeError) instead of broad catch-all.
CodeBleu
force-pushed
the
cloudstack_debian_fix
branch
from
fb1ccf2 to
1613efe
Compare
blackboxsw
added
incomplete
|
Add CLA-not-signed label, as we attempt to sort previously signed organization-wide agreement as well or determine that a new agreement is required. |
Any update on this? My company mentioned something being signed, but need this to validate based on obfuscated Github email, as I do have valid company email on my github account. I do not want to publish my real email. This should be doable, and hoping the cloud-init team in charge is able to find a solution for this. |
|
@CodeBleu thank you
I agree here too. This is quite a bit more painful than I would have hoped. I can confirm that our old internal CLA document represents you signing in 2023 for an org-wide approval. Unfortunately, I also see updates that a new contract was sent around November 24, 2025 that appears to be blocked on signature. I'm confirming internally whether this is blocked on your side our ours. |
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging blackboxsw, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag blackboxsw to reopen it.) |
github-actions
bot
added
the
stale-pr
|
@blackboxsw any update? I'd hate to see this get closed because the signing check needs adjusted. |
github-actions
bot
removed
the
stale-pr
github-actions
bot
added
the
stale-pr
|
@CodeBleu I reached out to the team responsible for our CLA service with the suggestion before end of year holidays. I've bumped this conversation again with the suggestion to use the GH API for organization lookup to allow us to validate github accounts with obscured email addresses by checking if those accounts are members of a known github organization which has signed the org-wide CLA. This is on our radar and we'll make sure we can approach this in a reasonable way, or find a way to establish an exception process in the meantime if it looks like this will take too long to obtain a solution. |
|
Hello @CodeBleu. My apologies for the delay here. I got confirmation from the team working on the CLA service, which checks only public email addresses at the moment, that this feature to allow obscured email addresses won't make it on the roadmap for the near future. Their suggestion for expedited approval would be to confirm with you organization whether invidividuals can sign given that the organization-wide CLA signature was approved for your company. Please report back if that is not possible. |
@blackboxsw This has already been done a while back, not sure what else you need. The powers that be at my org have already signed and confirmed things were good. If you have email or something of someone we need to reach out to, let me know. FYI - this experience has not been good and not gonna lie, makes me not want to contribute if I have to go through all of this. |
3 participants
Proposed Commit Message
Additional Context
Test Steps
Merge type