You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by GitBox <gi...@apache.org> on 2023/01/16 12:11:13 UTC
[GitHub] [incubator-uniffle] advancedxy opened a new pull request, #491: [operator] better build system support
advancedxy opened a new pull request, #491:
URL: https://github.com/apache/incubator-uniffle/pull/491
### What changes were proposed in this pull request?
1. image name defaults to rss-$module when no registry is specifed
2. upgrade envtest to 1.24.1 to support Apple M1 chips
3. modify Dockerfile to replace base image from alpine to debian slim
4. new changes in auto-generated code
5. remove gorequest deps
### Why are the changes needed?
to address #479.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
No need
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen merged pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen merged PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071785271
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
It seems that the names of individual fields in k8s v1.24 have been changed, and I recommend a separate PR to change the k8s version.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
zuston commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1073258082
##########
deploy/kubernetes/operator/pkg/webhook/util/util.go:
##########
@@ -192,16 +196,27 @@ func HasZeroApps(pod *corev1.Pod) bool {
return true
}
url := fmt.Sprintf("http://%v:%v/metrics/server", pod.Status.PodIP, port)
- req := gorequest.New().Timeout(time.Second * 15).Get(url).Type("json")
- resp, body, errs := req.EndBytes()
- if len(errs) > 0 {
- klog.Errorf("send metrics server request failed: %v->%+v", url, errs)
+ req, err := http.NewRequest("GET", url, nil)
Review Comment:
This change is for what?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071936670
##########
deploy/kubernetes/operator/api/uniffle/v1alpha1/zz_generated.deepcopy.go:
##########
@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
Review Comment:
This is due to locally dev machine's go version is 1.17 or higher(such as 1.19).
I will create a followup pr to upgrade the go version to 1.17.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089918217
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1385321720
@zuston could you also help review this pr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071742779
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
Actually my question is why update envtest?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089915154
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
Review Comment:
@kaijchen do you have more comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1073333557
##########
deploy/kubernetes/operator/pkg/webhook/util/util.go:
##########
@@ -192,16 +196,27 @@ func HasZeroApps(pod *corev1.Pod) bool {
return true
}
url := fmt.Sprintf("http://%v:%v/metrics/server", pod.Status.PodIP, port)
- req := gorequest.New().Timeout(time.Second * 15).Get(url).Type("json")
- resp, body, errs := req.EndBytes()
- if len(errs) > 0 {
- klog.Errorf("send metrics server request failed: %v->%+v", url, errs)
+ req, err := http.NewRequest("GET", url, nil)
Review Comment:
I removed the github.com/parnurzeal/gorequest lib, which may introduce curl cli deps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on pull request #491: [ISSUE-479] [operator] better build system support
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1407430353
Rerun CI to see if #512 can be fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071924464
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
> Actually my question is why update envtest?
To quote:`upgrade envtest to 1.24.1 to support Apple M1 chips`. envtest v1.22.x doesn't support M1 chip, not publish related arm archives. So, you cannot test the controller on M1.
The operator's required k8s in go.mod doesn't change, and still goes to 1.22 (k8s.io/api v0.22.x). And RSS's operator doesn't seem to use too much advanced features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
zuston commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1080754476
##########
deploy/kubernetes/operator/pkg/webhook/util/util.go:
##########
@@ -192,16 +196,27 @@ func HasZeroApps(pod *corev1.Pod) bool {
return true
}
url := fmt.Sprintf("http://%v:%v/metrics/server", pod.Status.PodIP, port)
- req := gorequest.New().Timeout(time.Second * 15).Get(url).Type("json")
- resp, body, errs := req.EndBytes()
- if len(errs) > 0 {
- klog.Errorf("send metrics server request failed: %v->%+v", url, errs)
+ req, err := http.NewRequest("GET", url, nil)
Review Comment:
Sounds good. I think the built-in golang http dep is enough. Right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089940447
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
> As discussed earlier, why upgrade envtest to 1.24 when you don't need to upgrade k8s to 1.24? cc @advancedxy
Yeah, as discussed earlier. Envtest is upgraded to 1.24 to support Apple M1's chips as 1.22 doesn't work for M1 chip.
The operator itself doesn't need to upgrade to 1.24 and it shouldn't since many K8S clusters are in old versions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089883780
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
Review Comment:
Is `ARG MODULE` repeated?
See line 21.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089916690
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
Review Comment:
Yes this looks good, ref: https://docs.docker.com/engine/reference/builder/#scope
Maybe add some empty lines or comments between two build stages, so it will be more clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071713944
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
Maybe you can update it in go.mod as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1384008668
> I have a question about this, when I built the rss image from the master branch, the image version is release.0.4.0.316.g01d37beb 
>
> This makes me confused.
Me too. I'm not sure why this version format is used. Maybe @wangao1236 could give us some hint.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071942203
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
WORKDIR /
COPY --from=builder /workspace/${MODULE} .
-
-CMD ["${MODULE}"]
Review Comment:
Previously, this is wrong:
1. the `ARG MODULE` is not defined in the second stage. the module might be empty or undefined.
2. MODULE is only available in build time. You cannot start the container by default since the `MODULE` env is not defined in the container.
cc @wangao1236
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1080756951
##########
deploy/kubernetes/operator/pkg/webhook/util/util.go:
##########
@@ -192,16 +196,27 @@ func HasZeroApps(pod *corev1.Pod) bool {
return true
}
url := fmt.Sprintf("http://%v:%v/metrics/server", pod.Status.PodIP, port)
- req := gorequest.New().Timeout(time.Second * 15).Get(url).Type("json")
- resp, body, errs := req.EndBytes()
- if len(errs) > 0 {
- klog.Errorf("send metrics server request failed: %v->%+v", url, errs)
+ req, err := http.NewRequest("GET", url, nil)
Review Comment:
yes. For simple cases, built-in http lib is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
zuston commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1385448647
> @zuston could you also help review this pr.
OK. I will take a look tomorrow, but to be honest, I don’t have much golang experience.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1407629913
ping @wangao1236 @kaijchen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1407584777
@wangao1236 @kaijchen please take a look at this. I would like this to be merged today as there are other changes based on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089884136
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
As discussed earlier, why upgrade envtest to 1.24 when you don't need to upgrade k8s to 1.24?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089982696
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,12 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+
+FROM debian:11.6-slim
+# docker's ARG scope doc: https://docs.docker.com/engine/reference/builder/#scope, it's required to declear an arg in
+# each stage.
+ARG MODULE
WORKDIR /
COPY --from=builder /workspace/${MODULE} .
-
-CMD ["${MODULE}"]
+ENV OPERATOR_BIN=/${MODULE}
+ENTRYPOINT "/${OPERATOR_BIN}"
Review Comment:
I suggest using exec mode in dockerfile. For example, `ENTRYPOINT ["${OPERATOR_BIN}"]`.
Ref: https://emmer.dev/blog/docker-shell-vs.-exec-form/
However, this does not affect normal running.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1090132177
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,12 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+
+FROM debian:11.6-slim
+# docker's ARG scope doc: https://docs.docker.com/engine/reference/builder/#scope, it's required to declear an arg in
+# each stage.
+ARG MODULE
WORKDIR /
COPY --from=builder /workspace/${MODULE} .
-
-CMD ["${MODULE}"]
+ENV OPERATOR_BIN=/${MODULE}
+ENTRYPOINT "/${OPERATOR_BIN}"
Review Comment:
Good suggestion. Let's raise a follow up pr to quick address it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089890768
##########
deploy/kubernetes/operator/hack/Dockerfile:
##########
@@ -35,12 +35,9 @@ COPY pkg/ pkg/
# Build
RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GO111MODULE=on go build -a -o ${MODULE} cmd/${MODULE}/main.go
-# Use distroless as minimal base image to package the manager binary
-# Refer to https://github.com/GoogleContainerTools/distroless for more details
-FROM alpine
-RUN apk add --update curl && \
- rm -rf /var/cache/apk/*
+FROM debian:11.6-slim
+ARG MODULE
Review Comment:
No. ARG should be declared multiple times per build stage. See https://stackoverflow.com/a/53682110
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] zuston commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
zuston commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1383983592
I have a question about this, when I built the rss image from the master branch, the image version is release.0.4.0.316.g01d37beb
This makes me confused.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on a diff in pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1071717380
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
which lib should I update the version? `k8s.io/api v0.22.2` -> what version?
I'm not quite family with envtest...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
wangao1236 commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1384017205
> > I have a question about this, when I built the rss image from the master branch, the image version is release.0.4.0.316.g01d37beb 
> > This makes me confused.
>
> Me too. I'm not sure why this version format is used. Maybe @wangao1236 could give us some hint.
This is just a way of differentiating versions. You can customize the version name by setting value of "IMAGE_VERSION". Maybe we can define a new way to differentiate versions. Do you have any suggestions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
advancedxy commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1383962943
@wangao1236 please take a look when you have time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #491: [operator] better build system support
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#issuecomment-1384781545
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#491](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71f3ebf) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/849993c26d06c422f38f0bb242af00fb2e341290?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (849993c) will **decrease** coverage by `0.64%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #491 +/- ##
============================================
- Coverage 58.83% 58.19% -0.65%
+ Complexity 1707 1488 -219
============================================
Files 206 184 -22
Lines 11508 9675 -1833
Branches 1030 871 -159
============================================
- Hits 6771 5630 -1141
+ Misses 4323 3674 -649
+ Partials 414 371 -43
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...storage/handler/impl/DataSkippableReadHandler.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmFnZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdW5pZmZsZS9zdG9yYWdlL2hhbmRsZXIvaW1wbC9EYXRhU2tpcHBhYmxlUmVhZEhhbmRsZXIuamF2YQ==) | `83.78% <0.00%> (-2.71%)` | :arrow_down: |
| [.../org/apache/spark/shuffle/writer/WriterBuffer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvV3JpdGVyQnVmZmVyLmphdmE=) | | |
| [...mapreduce/task/reduce/RssInMemoryRemoteMerger.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkdWNlL3Rhc2svcmVkdWNlL1Jzc0luTWVtb3J5UmVtb3RlTWVyZ2VyLmphdmE=) | | |
| [...org/apache/spark/shuffle/writer/AddBlockEvent.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvQWRkQmxvY2tFdmVudC5qYXZh) | | |
| [...pache/hadoop/mapreduce/task/reduce/RssShuffle.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkdWNlL3Rhc2svcmVkdWNlL1Jzc1NodWZmbGUuamF2YQ==) | | |
| [.../java/org/apache/hadoop/mapreduce/RssMRConfig.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkdWNlL1Jzc01SQ29uZmlnLmphdmE=) | | |
| [.../hadoop/mapreduce/task/reduce/RssEventFetcher.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkdWNlL3Rhc2svcmVkdWNlL1Jzc0V2ZW50RmV0Y2hlci5qYXZh) | | |
| [...java/org/apache/hadoop/mapred/SortWriteBuffer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkL1NvcnRXcml0ZUJ1ZmZlci5qYXZh) | | |
| [...apache/hadoop/mapreduce/v2/app/RssMRAppMaster.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LW1yL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvbWFwcmVkdWNlL3YyL2FwcC9Sc3NNUkFwcE1hc3Rlci5qYXZh) | | |
| [...che/spark/shuffle/writer/BufferManagerOptions.java](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvQnVmZmVyTWFuYWdlck9wdGlvbnMuamF2YQ==) | | |
| ... and [13 more](https://codecov.io/gh/apache/incubator-uniffle/pull/491?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089948794
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
> > As discussed earlier, why upgrade envtest to 1.24 when you don't need to upgrade k8s to 1.24? cc @advancedxy
>
> Yeah, as discussed earlier. Envtest is upgraded to 1.24 to support Apple M1's chips as 1.22 doesn't work for M1 chip.
>
> The operator itself doesn't need to upgrade to 1.24 and it shouldn't since many K8S clusters are in old versions.
Ok, I get it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] wangao1236 commented on a diff in pull request #491: [ISSUE-479] [operator] refine operator's build system
Posted by "wangao1236 (via GitHub)" <gi...@apache.org>.
wangao1236 commented on code in PR #491:
URL: https://github.com/apache/incubator-uniffle/pull/491#discussion_r1089884136
##########
deploy/kubernetes/operator/Makefile:
##########
@@ -16,12 +16,16 @@
#
# REGISTRY URL to use all building/pushing image targets
-REGISTRY ?= UNKNOWN_REGISTRY
+REGISTRY ?= ''
+
+ifneq ('', ${REGISTRY})
+REGISTRY := $(addsuffix /, ${REGISTRY})
+endif
MODULES ?= webhook controller
# ENVTEST_K8S_VERSION refers to the version of kubebuilder assets to be downloaded by envtest binary.
-ENVTEST_K8S_VERSION = 1.22.1
+ENVTEST_K8S_VERSION = 1.24.1
Review Comment:
As discussed earlier, why upgrade envtest to 1.24 when you don't need to upgrade k8s to 1.24?
cc @advancedxy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org