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

EDGECLOUD-5233 create appinst fails fqdn prefix invalid DNS name #1623

Merged
merged 1 commit into from Jul 9, 2021

Conversation

gainsley
Copy link
Contributor

@gainsley gainsley commented Jul 9, 2021

Issues Fixed

  • EDGECLOUD-5233 Creation of AppInst with user specified deployment manifest fails

Description

Changes corresponding to edge-cloud PR: mobiledgex/edge-cloud#1403

The changes here make it so CreateAppDNSAndPatchKubeSvc does not issue DNS records for kubernetes services that don't have their own IP. To remove the double negative: only kubernetes services that are issued their own IP for platforms like GCP/Azure/etc, will issue a DNS record for the service name (fqdn prefix + uri).

Copy link
Contributor

@jlmorris3827 jlmorris3827 left a comment

Choose a reason for hiding this comment

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

The code looks good. I can see new RootLBs will pickup the new naming convention.

But what do we do for existing cloudlets that rootLbs already running? We only create DNS records when the rootLB is created, if it already exists it will not execute the code that does the DNS record creation. We may need to either run the DNS record code for existing rootLbs on startup somehow, but ideally just once (might be hard to track that)

@gainsley
Copy link
Contributor Author

gainsley commented Jul 9, 2021

The code looks good. I can see new RootLBs will pickup the new naming convention.

But what do we do for existing cloudlets that rootLbs already running? We only create DNS records when the rootLB is created, if it already exists it will not execute the code that does the DNS record creation. We may need to either run the DNS record code for existing rootLbs on startup somehow, but ideally just once (might be hard to track that)

Actually Jim, I believe it's ok. Vmlayer platform Init() calls SetupRootLB whether or not the rootLB already existed, and that updates the DNS entry. So if it's an old CRM, based on the compatibility version, the Controller will continue to use the old DNS name in the URI. If it's a new CRM, then when the CRM is started, it will update the DNS entry to the new name, and the Controller will start using the new name in the URI.

@jlmorris3827
Copy link
Contributor

jlmorris3827 commented Jul 9, 2021

SetupRootLB

oh you're right, I was thinking the code exited before doing this, but I guess not. So the only question is dedicated LBs, but maybe that is not worth worrying about as the big majority of the time the AppInst is created at the same time as the cluster.

edit dedicated format isn't changing anyway and so doesn't matter

@gainsley
Copy link
Contributor Author

gainsley commented Jul 9, 2021

Cool, thanks for reviewing Jim!

@gainsley gainsley merged commit d22fe24 into master Jul 9, 2021
@gainsley gainsley deleted the fqdnprefix branch July 9, 2021 17:45
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.

None yet

3 participants