-
Notifications
You must be signed in to change notification settings - Fork 176
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
Managed CSC for in-cluster CSM #1674
base: main
Are you sure you want to change the base?
Conversation
/gcbrun |
3ee6d4c
to
377e116
Compare
/gcbrun |
1 similar comment
/gcbrun |
/gcbrun |
/gcbrun |
@@ -266,7 +278,5 @@ install_control_plane() { | |||
if use_fleet_api; then install_fleet_api; else install_control_plane_revision; fi | |||
fi | |||
|
|||
if [[ "$DISABLE_CANONICAL_SERVICE" -eq 0 ]] && ! is_managed; then |
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 need to install managed CSC only when in-cluster CSC is not present, we will need to check for existence of in-cluster CSC.
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 see that we are doing this in install_managed_canonical_controller but we need to do kubectl apply -f "${CANONICAL_CONTROLLER_MANIFEST}"
if in-cluster CP exists if there is change in in-cluster csc manifest.
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.
Moved the logic to update the existing in-cluster CSC Deployment and checking the managed CSC status under one routine "verify_canonical_controller".
asmcli/commands/install.sh
Outdated
local IN_CLUSTER_CSC_DEP; IN_CLUSTER_CSC_DEP="$(kubectl get deployment/canonical-service-controller-manager \ | ||
-n asm-system --ignore-not-found=true || true)" | ||
if [[ -z "$IN_CLUSTER_CSC_DEP" ]]; then | ||
enable_service_mesh_feature |
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.
Let's enable service mesh feature before control plane installation itself so that it can be used by other functions in future and we get sufficient time also for managed CSC to be enabled.
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.
Agree. Enabled service mesh in validate_dependencies() in commands/validate.sh. Once Chris replies, there will more clarity on what conditions to add before enabling the service mesh feature. For now enabled it for scenarios:
- All off-GCP platforms
- GCP platform where we can modify gcp components
asmcli/commands/install.sh
Outdated
@@ -101,6 +101,18 @@ install_canonical_controller() { | |||
info "...done!" | |||
} | |||
|
|||
install_managed_canonical_controller() { |
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.
nit enable_managed_canonical_controller
instead of install_managed_canonical_controller
?
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.
Agree. We are not installing the managed CSC here. Here renamed the routine "install_canonical_controller" to "verify_canonical_controller" as we will be just checking the state of managed CSC or updating the existing in-cluster CSC deployment
34f09f1
to
7259b23
Compare
7259b23
to
f8e82a2
Compare
@@ -64,8 +62,9 @@ validate_dependencies() { | |||
exit_if_service_mesh_feature_not_enabled | |||
fi | |||
fi | |||
else | |||
enable_service_mesh_feature |
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.
This will enable_service_mesh_feature for offGCP platforms in asmcli validate command also.
So asmcli install and asmcli validate both calls validate_dependencies. in asmcli validate command user can not pass --enable*
flags so can_modify_gcp_components gives false and we exit_if_service_mesh_feature_not_enabled.
Now I think we can move out the logic from is_gcp section and add something like
if can_modify_gcp_components
enable_service_mesh_feature
else
exit_if_service_mesh_feature_not_enabled
So now we will need extra enable_gcp_components
flag also for offGCP platforms.
Install Managed Canonical Service controller for in-cluster CSM.