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

Automatically fetch config map values #1199

Closed
wants to merge 2 commits into from

Conversation

esposem
Copy link
Contributor

@esposem esposem commented Jul 14, 2023

This PR gets rid of some ConfigMap parameters that must be provided to CAA in order to make it run on Azure/AWS.

More specifically:

  • In Azure, ConfigMap was
    cat <<EOF | oc apply -f -
apiVersion: v1
kind: ConfigMap
metadata:
  name: ${CM}
  namespace: ${OP_NAME}
data:
  CLOUD_PROVIDER: "azure"
  VXLAN_PORT: "9000"
  AZURE_INSTANCE_SIZE: "Standard_D8as_v5" #set
  PROXY_TIMEOUT: "15m"
  AZURE_IMAGE_ID: ${AZURE_IMAGE_ID}
  AZURE_SUBNET_ID: ${AZURE_SUBNET_ID}
  AZURE_NSG_ID: ${AZURE_NSG_ID}
  DISABLECVM: "true"
EOF

and becomes now

    cat <<EOF | oc apply -f -
apiVersion: v1
kind: ConfigMap
metadata:
  name: ${CM}
  namespace: ${OP_NAME}
data:
  CLOUD_PROVIDER: "azure"
  VXLAN_PORT: "9000"
  AZURE_INSTANCE_SIZE: "Standard_D8as_v5" #set
  PROXY_TIMEOUT: "15m"
  AZURE_IMAGE_ID: ${AZURE_IMAGE_ID}
  DISABLECVM: "true"
EOF

Current limitations of this patch

azure:

  • tested on Azure

Fixes: #1200

@esposem esposem force-pushed the configmap branch 2 times, most recently from 84c3bda to 6657690 Compare July 31, 2023 15:12
@esposem
Copy link
Contributor Author

esposem commented Jul 31, 2023

@snir911 @bpradipt the solution to the issues mentioned in the PR description are:

  • Assume the user provided a AZURE_USER_RESOURCE_GROUP field in the peerpodssecret, which corresponds to the user-created azure RG. This is valid only when using ARO/AKS.
  • Query for subnets/vnets/nsg but if more than one result is returned, then return an error and force the user to insert it via the ConfigMap
  • This implies (and should be documented somewhere) that for ARO the subnet ID must be provided by the user, as workers and masters are in two different user-created subnets, and we need the worker subnet.

@esposem esposem force-pushed the configmap branch 2 times, most recently from 90cf934 to b541470 Compare July 31, 2023 15:31
Copy link
Contributor

@snir911 snir911 left a comment

Choose a reason for hiding this comment

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

Thanks! i added some comments, also please look at the build and lint actions

pkg/adaptor/cloud/azure/provider.go Show resolved Hide resolved
pkg/adaptor/cloud/azure/provider.go Show resolved Hide resolved
pkg/adaptor/cloud/azure/provider.go Show resolved Hide resolved
entrypoint.sh Show resolved Hide resolved
pkg/adaptor/cloud/azure/manager.go Outdated Show resolved Hide resolved
@esposem esposem force-pushed the configmap branch 4 times, most recently from e8c535b to b7e5a8a Compare August 8, 2023 06:57
@esposem esposem requested a review from snir911 August 24, 2023 12:34
@esposem
Copy link
Contributor Author

esposem commented Aug 24, 2023

Updated to work only on Azure. Managed services like ARO and AKS are left as TODO, since they have different resource groups and the Azure SDK API might not fully support them yet.

@esposem esposem changed the title Draft: automatically fetch config map values Automatically fetch config map values Aug 30, 2023
@bpradipt
Copy link
Member

bpradipt commented Oct 4, 2023

@esposem can you please rebase this PR ?

It is used nowhere.

Fixes: confidential-containers#1200

Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
@esposem
Copy link
Contributor Author

esposem commented Oct 9, 2023

@bpradipt I rebased, but it looks like we need more work for this? I understood that this change was not needed if we use IMDS... As @snir911 suggested, maybe I need to close this PR?

Using the Azure SDK, we don't need to provide AZURE_IMAGE_ID,
AZURE_SUBNET_ID and AZURE_NSG_ID to the ConfigMap.

If each query returns more than a result, return an error and let
the user specify the actual value needed in the ConfigMap.

Fixes: confidential-containers#1200

Signed-off-by: Emanuele Giuseppe Esposito <[email protected]>
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.

automatically fetch config map values
3 participants