-
Notifications
You must be signed in to change notification settings - Fork 57
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
VPC Support in AKO #1416
VPC Support in AKO #1416
Conversation
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
485c9bb
to
d8b294f
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
1 similar comment
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
1fa2ece
to
281722b
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
281722b
to
1ddc34e
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
1 similar comment
build ako |
1ddc34e
to
df8276b
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
skip jira-id-check |
build ako |
df8276b
to
1b1e1d7
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
1b1e1d7
to
f7c5437
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
f7c5437
to
1f32ac6
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
skip jira-id-check |
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
1f32ac6
to
d653c48
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
1 similar comment
build ako |
d653c48
to
38871c8
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
1 similar comment
build ako |
38871c8
to
b102030
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
b102030
to
341d5db
Compare
No JIRA Ids found for the PR. Jira id is mandatory to update fix version in jira. Please update respective Jira id in PR title or commit message if the PR is intented for default branches of repo. For mandatory exemptions, comment trigger phrase 'skip jira-id-check' in PR. For manual trigger, comment trigger phrase 'run jira-id-check' in PR. |
build ako |
2 similar comments
build ako |
build ako |
@@ -359,6 +359,9 @@ func (l *leader) ValidateAviInfraSetting(key string, infraSetting *akov1beta1.Av | |||
if infraSetting.Spec.SeGroup.Name != "" { | |||
refData[infraSetting.Spec.SeGroup.Name] = "ServiceEngineGroup" | |||
} | |||
if infraSetting.Spec.Tenant.Name != nil && !utils.IsVCFCluster() { | |||
refData[*infraSetting.Spec.Tenant.Name] = "Tenant" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think presense of tenant ref should be validated only in admin tenant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presently, every ref in CRs is validated in Admin tenant only. Will handle CRD validation in subsequent PRs
build ako |
1 similar comment
build ako |
akoControlConfig.SetAKOInstanceFlag(true) | ||
lib.SetNamePrefix("") | ||
lib.SetAKOUser(lib.AKOPrefix) | ||
//utils.AviLog.SetLevel("INFO") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove commented code.
ctrlPropCache := utils.SharedCtrlProp() | ||
ctrlPropCache.PopulateCtrlProp(ctrlProps) | ||
if cfg.useEnvoy { | ||
ctrlPropCache.PopulateCtrlAPIScheme("http") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be https
for secure connection?
flag.StringVar(&password, "password", "", "NSX ALB Controller authentication password") | ||
flag.StringVar(&albCtrlCert, "cacert", "", "NSX ALB Controller authentication certificate") | ||
flag.StringVar(&clusterID, "cluster-id", "", "AKO cluster name") | ||
flag.BoolVar(&useEnvoy, "use-envoy", false, "Use Envoy sidecar proxy in VCSA") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is use of envoy?
|
||
ctx, cancelFunc := context.WithTimeout(context.Background(), 120*time.Second) | ||
defer cancelFunc() | ||
err := cfg.Cleanup(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are we deleting all avi resources? There is deleteConfig
functionality present in AKO. Can we use that? It deletes all resources created by current AKO. Can we use that?
|
||
func getInfraSettingNameFromT1LR(lr string) string { | ||
arr := strings.Split(lr, "/") | ||
infraSettingName := arr[len(arr)-1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume lr
has specific format and string split should result in non-zero len array. Else this line will cause index out of range error.
if len(infraSettingCRs) == 0 { | ||
return nil | ||
} | ||
namespaces, err := utils.GetInformers().NSInformer.Lister().List(labels.Set(nil).AsSelector()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we maintain map of aviinfrasetting to namespace during namespace event handler or other add event so that it will be faster to delete not required entries?
} | ||
} | ||
|
||
func (v *VPCHandler) SyncLSLRNetwork() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are multiple SyncLSLRNetwork
functions (one in VCF context and one in VPC context) to update cloud with tier1lr
. Will it possible that it will create conflict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, only one will be enabled due lib.IsVPCmode()
check.
if err != nil { | ||
utils.AviLog.Fatalf("Error building AKO CRD v1beta1 clientset: %s", err.Error()) | ||
} | ||
|
||
utils.AviLog.Infof("Successfully created kube client for ako-infra") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can move this message before line number 77.
lib.AKOControlConfig().SetControllerVersion(currentControllerVersion) | ||
ctrlVersion = currentControllerVersion | ||
} | ||
// set the tenant and controller version in avisession obj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment should be modified as we are setting up controller version only.
err = c.AviCloudPropertiesPopulate(client[0], cloud) | ||
if err != nil { | ||
return vsCacheCopy, allVsKeys, err | ||
} | ||
} | ||
if lib.GetDeleteConfigMap() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arihantg: We should test deletConfig
functionality with tenancy into picture?
The changes have been moved to PR #1446 |
Overview of behaviour changes and how they are implemented - https://confluence.eng.vmware.com/display/NSBU/AKO+Changes+to+support+NSX+VPCs