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.

Sign up for GitHub

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

Support package refactor #336

Merged
merged 6 commits into from
Oct 24, 2023
Merged

Support package refactor #336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
86164e7
removing support package
Fiona-Waters Oct 13, 2023
b938a58
updating support package import
Fiona-Waters Oct 13, 2023
3b2f041
removing defaults make target
Fiona-Waters Oct 16, 2023
32c2dea
updating files
Fiona-Waters Oct 23, 2023
8635412
update go.mod and go.sum
Fiona-Waters Oct 24, 2023
5bb981a
addressing feedback
Fiona-Waters Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/tag-and-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,8 @@ jobs:
- name: Adjust MCAD, SDK and InstaScale dependencies in the code
run: |
# Remove leading 'v'
CODEFLARE_SDK_VERSION=$(cut -c2- <<< ${{ github.event.inputs.codeflare-sdk-version }})
sed -i -E "s/(.*MCAD_VERSION \?= ).*/\1${{ github.event.inputs.mcad-version }}/" Makefile
sed -i -E "s/(.*MCAD_REF \?= ).*/\1release-\${MCAD_VERSION}/" Makefile
sed -i -E "s/(.*CODEFLARE_SDK_VERSION \?= ).*/\1$CODEFLARE_SDK_VERSION/" Makefile
sed -i -E "s/(.*INSTASCALE_VERSION \?= ).*/\1${{ github.event.inputs.instascale-version }}/" Makefile

- name: Login to Quay.io
Expand Down
37 changes: 5 additions & 32 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ KUBERAY_VERSION ?= v1.0.0-rc.1
# RAY_VERSION defines the default version of Ray (used for testing)
RAY_VERSION ?= 2.5.0

# CODEFLARE_SDK_VERSION defines the default version of the CodeFlare SDK
CODEFLARE_SDK_VERSION ?= 0.10.1

# OPERATORS_REPO_ORG points to GitHub repository organization where bundle PR is opened against
# OPERATORS_REPO_FORK_ORG points to GitHub repository fork organization where bundle build is pushed to
OPERATORS_REPO_ORG ?= redhat-openshift-ecosystem
Expand Down Expand Up @@ -64,9 +61,6 @@ IMAGE_ORG_BASE ?= quay.io/project-codeflare
# codeflare.dev/codeflare-operator-bundle:$VERSION and codeflare.dev/codeflare-operator-catalog:$VERSION.
IMAGE_TAG_BASE ?= $(IMAGE_ORG_BASE)/codeflare-operator

# RAY_IMAGE defines the default container image for Ray (used for testing)
RAY_IMAGE ?= rayproject/ray:$(RAY_VERSION)

# BUNDLE_IMG defines the image:tag used for the bundle.
# You can use it as an arg. (E.g make bundle-build BUNDLE_IMG=<some-registry>/<project-name-bundle>:<tag>)
BUNDLE_IMG ?= $(IMAGE_TAG_BASE)-bundle:$(VERSION)
Expand Down Expand Up @@ -125,27 +119,6 @@ help: ## Display this help.

##@ Development

DEFAULTS_TEST_FILE := test/support/defaults.go

.PHONY: defaults
defaults:
$(info Regenerating $(DEFAULTS_TEST_FILE))
@echo "package support" > $(DEFAULTS_TEST_FILE)
@echo "" >> $(DEFAULTS_TEST_FILE)
@echo "// ***********************" >> $(DEFAULTS_TEST_FILE)
@echo "// DO NOT EDIT THIS FILE" >> $(DEFAULTS_TEST_FILE)
@echo "// ***********************" >> $(DEFAULTS_TEST_FILE)
@echo "" >> $(DEFAULTS_TEST_FILE)
@echo "const (" >> $(DEFAULTS_TEST_FILE)
@echo " CodeFlareSDKVersion = \"$(CODEFLARE_SDK_VERSION)\"" >> $(DEFAULTS_TEST_FILE)
@echo " RayVersion = \"$(RAY_VERSION)\"" >> $(DEFAULTS_TEST_FILE)
@echo " RayImage = \"$(RAY_IMAGE)\"" >> $(DEFAULTS_TEST_FILE)
Comment on lines -140 to -142
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also remove CODEFLARE_SDK_VERSION, RAY_VERSION and RAY_IMAGE references?
AFAIK they are used here, CODEFLARE_SDK_VERSION is also set in

codeflare-operator/.github/workflows/tag-and-build.yml

Line 96 in 9912d96

CODEFLARE_SDK_VERSION=$(cut -c2- <<< ${{ github.event.inputs.codeflare-sdk-version }})

Copy link
Contributor

Choose a reason for hiding this comment

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

That brings another point - should the codeflare-common repo be updated for every release (as it reference SDK version)?
IMHO SDK reference is just temporary, to be removed once project-codeflare/codeflare-sdk#292 is implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. I have removed those references.
Once that SDK PR is merged we can remove the references to its version in codeflare-common. I can create a follow on issue for this.
The issue is blocked at the moment so I it should probably be updated for any releases in the meantime?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Follow up issue: project-codeflare/codeflare-common#5
What do we need to do to ensure it is updated in the meantime?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am thinking whether it has sense to compose the codeflare-common update into release workflow...
That would be ideal solution but quite a big work considering that it would be removed soon. I would say that the reference can be updated manually in meantime until the SDK tests are implemented.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though the MCAD is still referenced in codeflare-common too...
So in the end it may have sense to integrate it in release workflow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Do we need to create an issue or track this anywhere?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, issue will be needed.
I will create it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Created - project-codeflare/codeflare-common#6

@echo "" >> $(DEFAULTS_TEST_FILE)
@echo ")" >> $(DEFAULTS_TEST_FILE)
@echo "" >> $(DEFAULTS_TEST_FILE)

gofmt -w $(DEFAULTS_TEST_FILE)

# this encounters sed issues on MacOS, quick fix is to use gsed or to escape the parentheses i.e. \( \)
.PHONY: manifests
manifests: controller-gen kustomize ## Generate RBAC objects.
Expand All @@ -172,11 +145,11 @@ modules: ## Update Go dependencies.
go mod tidy

.PHONY: build
build: modules defaults fmt vet ## Build manager binary.
build: modules fmt vet ## Build manager binary.
go build -o bin/manager main.go

.PHONY: run
run: modules defaults manifests fmt vet ## Run a controller from your host.
run: modules manifests fmt vet ## Run a controller from your host.
go run ./main.go

.PHONY: image-build
Expand Down Expand Up @@ -283,7 +256,7 @@ validate-bundle: install-operator-sdk
$(OPERATOR_SDK) bundle validate ./bundle --select-optional suite=operatorframework

.PHONY: bundle
bundle: defaults manifests kustomize install-operator-sdk ## Generate bundle manifests and metadata, then validate generated files.
bundle: manifests kustomize install-operator-sdk ## Generate bundle manifests and metadata, then validate generated files.
$(OPERATOR_SDK) generate kustomize manifests -q
cd config/manager && $(KUSTOMIZE) edit set image controller=$(IMG)
cd config/manifests && $(KUSTOMIZE) edit add patch --patch '[{"op":"add", "path":"/metadata/annotations/containerImage", "value": "$(IMG)" }]' --kind ClusterServiceVersion
Expand Down Expand Up @@ -362,11 +335,11 @@ catalog-push: ## Push a catalog image.
podman push $(CATALOG_IMG) $(CATALOG_PUSH_OPT)

.PHONY: test-unit
test-unit: defaults manifests fmt vet envtest ## Run unit tests.
test-unit: manifests fmt vet envtest ## Run unit tests.
KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $(go list ./... | grep -v /test/) -coverprofile cover.out

.PHONY: test-e2e
test-e2e: defaults manifests fmt vet ## Run e2e tests.
test-e2e: manifests fmt vet ## Run e2e tests.
go test -timeout 30m -v ./test/e2e

.PHONY: kind-e2e
Expand Down
5 changes: 3 additions & 2 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ go 1.19

require (
github.com/onsi/gomega v1.27.10
github.com/openshift-online/ocm-sdk-go v0.1.368
github.com/openshift/api v0.0.0-20230213134911-7ba313770556
github.com/openshift/client-go v0.0.0-20221019143426-16aed247da5c
github.com/project-codeflare/codeflare-common v0.0.0-20231023092720-93d03492db16
github.com/project-codeflare/instascale v0.2.1
github.com/project-codeflare/multi-cluster-app-dispatcher v1.37.1
github.com/ray-project/kuberay/ray-operator v0.0.0-20231016183545-097828931d15
Expand Down Expand Up @@ -70,6 +69,8 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/openshift-online/ocm-sdk-go v0.1.368 // indirect
github.com/openshift/client-go v0.0.0-20221019143426-16aed247da5c // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_golang v1.14.0 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/project-codeflare/codeflare-common v0.0.0-20231023092720-93d03492db16 h1:TRMLDP6IYt0CAd3+BkvY/r2lkpjI3sOsxf3tnQojZ9k=
github.com/project-codeflare/codeflare-common v0.0.0-20231023092720-93d03492db16/go.mod h1:zdi2GCYJX+QyxFWyCLMoTme3NMz/aucWDJWMqKfigxk=
github.com/project-codeflare/instascale v0.2.1 h1:t+Ax/sk4yPQznO6N+U8Cq+bk3afCcZDj9wnHfiGSDBg=
github.com/project-codeflare/instascale v0.2.1/go.mod h1:zSzBTP4cFkg+bD4JyYTDmDnGwVKY/+6ACks57NAiscc=
github.com/project-codeflare/multi-cluster-app-dispatcher v1.37.1 h1:hZhGwKTPeHYYhNbvO27NOjozVpy7m3I3apKf81u9U3A=
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/instascale_app_wrapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ limitations under the License.
package e2e

import (
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

func instaScaleJobAppWrapper(test Test, namespace *corev1.Namespace, config *corev1.ConfigMap) *mcadv1beta1.AppWrapper {
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/instascale_machinepool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ import (
"testing"

. "github.com/onsi/gomega"
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

func TestInstascaleMachinePool(t *testing.T) {
Expand Down
5 changes: 2 additions & 3 deletions test/e2e/instascale_machineset_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import (
"testing"

. "github.com/onsi/gomega"
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

func TestInstascaleMachineSet(t *testing.T) {
Expand All @@ -31,7 +30,7 @@ func TestInstascaleMachineSet(t *testing.T) {
"mnist.py": ReadFile(test, "mnist.py"),
})

// // Setup batch job and AppWrapper
// Setup batch job and AppWrapper
aw := instaScaleJobAppWrapper(test, namespace, cm)

// look for machine set with aw name - expect to find it
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/mnist_pytorch_mcad_job_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,13 @@ import (
"testing"

. "github.com/onsi/gomega"
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

// Trains the MNIST dataset as a batch Job managed by MCAD, and asserts successful completion of the training job.
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/mnist_raycluster_sdk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,14 @@ import (
"testing"

. "github.com/onsi/gomega"
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

// Creates a Ray cluster, and trains the MNIST dataset using the CodeFlare SDK.
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/mnist_rayjob_mcad_raycluster_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ import (
"testing"

. "github.com/onsi/gomega"
. "github.com/project-codeflare/codeflare-common/support"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
rayv1 "github.com/ray-project/kuberay/ray-operator/apis/ray/v1"

corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

// Trains the MNIST dataset as a RayJob, executed by a Ray cluster managed by MCAD,
Expand Down
3 changes: 1 addition & 2 deletions test/e2e/support.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import (
"embed"

"github.com/onsi/gomega"

"github.com/project-codeflare/codeflare-operator/test/support"
"github.com/project-codeflare/codeflare-common/support"
)

//go:embed *.py *.txt
Expand Down
37 changes: 0 additions & 37 deletions test/support/batch.go
View file
Open in desktop

This file was deleted.

Loading