gardener / machine-controller-manager

Declarative way of managing machines for Kubernetes cluster
Apache License 2.0
257 stars 117 forks source link

Update provider-mcm `Makefile` to remove dynamic variables #846

Closed himanshu-kun closed 1 year ago

himanshu-kun commented 1 year ago

How to categorize this issue?

/area dev-productivity /kind enhancement /priority 3

What would you like to be added:

Currently the Makefile in mcm-provider repos have the following variables:

CONTROL_NAMESPACE   :=
CONTROL_KUBECONFIG  := 
TARGET_KUBECONFIG   := 

# Below ones are used in tests
MACHINECLASS_V1     := dev/machineclassv1.yaml
MACHINECLASS_V2     := 
MCM_IMAGE           := 
MC_IMAGE            := 
# MCM_IMAGE         := eu.gcr.io/gardener-project/gardener/machine-controller-manager:v0.39.0
# MC_IMAGE          := $(IMAGE_REPOSITORY):v0.7.0
LEADER_ELECT        := "true"
# If Integration Test Suite is to be run locally against clusters then export the below variable
# with MCM deployment name in the cluster
MACHINE_CONTROLLER_MANAGER_DEPLOYMENT_NAME := machine-controller-manager

These variables have to be initialized by updating the Makefile to run MCM-provider locally . This leads to sometimes pushing of changes in Makefile as part of the PR. We cannot git ignore the Makefile as well (in case some make rules have to be updated, then it'll cause problems)

The Makefile should only have static content and the dynamic content should be moved to another file

Further the PR https://github.com/gardener/machine-controller-manager/pull/845 has now modified the way local-mcm-setting-up was done . In order to use the newly introduced make targets by this PR in machine-controller-manager repo , mcm-provider maintainers should adapt Makefile to read dynamic content from .env file

Following is the list of providers where we'll make changes:

Why is this needed:

For dev productivity

himanshu-kun commented 1 year ago

/assign

unmarshall commented 1 year ago

For azure this is already been done as part of https://github.com/gardener/machine-controller-manager-provider-azure/pull/105 (in the new code line). However if you wish to make changes to the existing master code line then it requires another PR.

himanshu-kun commented 1 year ago

For azure this is already been done as part of https://github.com/gardener/machine-controller-manager-provider-azure/pull/105 (in the new code line). However if you wish to make changes to the existing master code line then it requires another PR.

No its fine, thanks for letting me know