vmware / antrea-operator-for-kubernetes

Antrea Operator for Kubernetes deployments
Other
15 stars 12 forks source link

Uniform tool installation in Makefile #94

Closed antoninbas closed 8 months ago

antoninbas commented 9 months ago

Use a uniform installation process for golangci-lint, controller-gen and kustomize.

The process ensures that the correct version of the tool is used for every "build", regardless of what other version may already be installed on the system.

For kustomize, we ensure that the Makefile is the single source of truth, and we remove hack/verify-kustomize.sh.

ksamoray commented 9 months ago

Looks good, but when I run this antrea-manifest/antrea.yml shows the following diff - is this expected? :

 apiVersion: v1
 data:
   antrea-agent.conf: |
-    {{- .AntreaAgentConfig | nindent 4 }}
+{{- .AntreaAgentConfig | nindent 4 }}
   antrea-cni.conflist: |
-    {{- .AntreaCNIConfig | nindent 4 }}
+{{- .AntreaCNIConfig | nindent 4 }}
   antrea-controller.conf: |
-    {{- .AntreaControllerConfig | nindent 4 }}
+{{- .AntreaControllerConfig | nindent 4 }}
antoninbas commented 9 months ago

@ksamoray definitely not, and that's what I am trying to avoid. I am puzzled as to why it keeps removing the indentation for you.

Are you running on Linux or macOS?

Also could you run make clean && make manifests and share the output so I can double check?

ksamoray commented 9 months ago

Hi, I'm running on Ubuntu 22.04. Output is same as above after make clean && make manifests

antoninbas commented 8 months ago

Can you share the output of make clean && make manifests, i.e. what's displayed in your terminal?

ksamoray commented 8 months ago

Sure:

$ make clean && make manifests
===> Installing Controller-gen  <===
GOBIN=/home/ksamoray/code/antrea-operator-for-kubernetes/.controller-gen/v0.6.2 go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.2
===> Installing Kustomize <===
v5.2.1
kustomize installed to /home/ksamoray/code/antrea-operator-for-kubernetes/.kustomize/5.2.1/kustomize
KUSTOMIZE=/home/ksamoray/code/antrea-operator-for-kubernetes/.kustomize/5.2.1/kustomize ./hack/generate-antrea-resources.sh --platform kubernetes --version 1.14.1
cp ./config/rbac/role.yaml ./deploy/kubernetes/role.yaml
cp ./config/samples/operator_v1_antreainstall.yaml ./deploy/kubernetes/operator.antrea.vmware.com_v1_antreainstall_cr.yaml
/home/ksamoray/code/antrea-operator-for-kubernetes/.controller-gen/v0.6.2/controller-gen "crd:trivialVersions=true" rbac:roleName=antrea-operator webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/home/ksamoray/code/antrea-operator-for-kubernetes/.kustomize/5.2.1/kustomize build config/crd > config/crd/operator.antrea.vmware.com_antreainstalls.yaml
antoninbas commented 8 months ago

@ksamoray found the issue: there was a sed command with a substitution that only worked for GNU sed, not with macOS sed. The substitution was not needed, so I removed it.

antoninbas commented 8 months ago

@ksamoray I was planning on adding (in a later PR) a Github workflow to check that the files have been generated correctly for each PR. The only issue remaining is that we keep flip-flopping between kubernetes and openshift in the manifest:

-  antreaImage: antrea/antrea-ubi:v1.14.1
-  antreaPlatform: openshift
+  antreaImage: antrea/antrea-ubuntu:v1.14.1
+  antreaPlatform: kubernetes

Running make manifests will default to kubernetes, but when you prepare a release for openshift, it changes. A simple solution is to just default to openshift / UBI for make manifests. Another solution is to reset changes to the manifest after preparing the release (I think you use make ocpbundle for that?). Do you have a preference?

The workflow would just validate that the file is correct / up-to-date. Having workflows generate and commit code is often not a good idea.

ksamoray commented 8 months ago

@antoninbas in which scenario do we need/use the kubernetes bundle? AFAIK we do not certify anywhere other the OpenShift right now, so maybe we can default to UBI but leave the kubernetes support intact in case we need it?

antoninbas commented 8 months ago

Honestly I don't even know if the terms bundle and certification are appropriate to use in the kubernetes case, or if they are specific to OpenShift?

While the operator could be a valid way to install Antrea on a vanilla K8s cluster, I am not aware of anyone using it for this purpose. With the lack of user-facing documentation (and the lack of published release artifacts?) on how to use the operator outside of Openshift, it's a bit hard to consume it in that fashion too.

So I am fine with defaulting ANTREA_PLATFORM to openshift. If someone wants to generate files for Kubernetes, they will just need to explicitly set ANTREA_PLATFORM=kubernetes. Maybe in the future, documentation & usability can be improved for the vanilla Kubernetes (non-Openshift) case?

ksamoray commented 8 months ago

For sure we can improve the process and the documentation as well. IMO we could store kubernetes and openshift bundles in two separate directories (and maybe avoid storing the interim files which are modified during the process as we do today). Is it technically feasible to to this process in a GitHub action?

antoninbas commented 8 months ago

@ksamoray definitely possible. IMO it's usually better to have a single Makefile target that takes care of all code generation (e.g., could generate both bundle versions on our case), and have a Github workflow validate that the code is up-to-date for each PR. And yes, if we have intermediate files which are not useful to end users, it may be better to remove them from the repo.