-
Notifications
You must be signed in to change notification settings - Fork 380
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
feat: Recommendation framework #445
Conversation
…ommendation via framework(WIP)
/hold |
♻️ PR Preview b7228dd has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
done. pls review this pr @qmhu |
🎉 Successfully Build Images. Docker RegistryOverview: https://hub.docker.com/u/gocrane
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=gocrane/craned \
--set craned.image.tag=pr-445-b7228dd \
--set metricAdapter.image.repository=gocrane/metric-adapter \
--set metricAdapter.image.tag=pr-445-b7228dd \
--set craneAgent.image.repository=gocrane/crane-agent \
--set craneAgent.image.tag=pr-445-b7228dd \
--set cranedDashboard.image.repository=gocrane/dashboard \
--set cranedDashboard.image.tag=pr-445-b7228dd crane/crane Coding RegistryOverview: https://finops.coding.net/public-artifacts/gocrane/crane/packages
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=finops-docker.pkg.coding.net/gocrane/crane/craned \
--set craned.image.tag=pr-445-b7228dd \
--set metricAdapter.image.repository=finops-docker.pkg.coding.net/gocrane/crane/metric-adapter \
--set metricAdapter.image.tag=pr-445-b7228dd \
--set craneAgent.image.repository=finops-docker.pkg.coding.net/gocrane/crane/crane-agent \
--set craneAgent.image.tag=pr-445-b7228dd \
--set cranedDashboard.image.repository=finops-docker.pkg.coding.net/gocrane/crane/dashboard \
--set cranedDashboard.image.tag=pr-445-b7228dd crane/crane Ghcr RegistryOverview: https://github.com/orgs/gocrane/packages?repo_name=crane
Quick Deploy - Helm helm repo add crane https://finops-helm.pkg.coding.net/gocrane/gocrane
helm install crane -n crane-system --create-namespace \
--set craned.image.repository=ghcr.io/gocrane/crane/craned \
--set craned.image.tag=pr-445-b7228dd \
--set metricAdapter.image.repository=ghcr.io/gocrane/crane/metric-adapter \
--set metricAdapter.image.tag=pr-445-b7228dd \
--set craneAgent.image.repository=ghcr.io/gocrane/crane/crane-agent \
--set craneAgent.image.tag=pr-445-b7228dd \
--set cranedDashboard.image.repository=ghcr.io/gocrane/crane/dashboard \
--set cranedDashboard.image.tag=pr-445-b7228dd crane/crane |
func (rr *ReplicasRecommender) PreRecommend(ctx *framework.RecommendationContext) error { | ||
// we load algorithm config in this phase | ||
// TODO(chrisydxie) support configuration | ||
config := &config.Config{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we have multiple algo config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe the specific user case? Do we need hyper-parameter search?
23e7ab6
to
e5ed0a3
Compare
@@ -73,3 +79,33 @@ func NewRecommendationContext(context context.Context, identity analytics.Object | |||
// return false | |||
// } | |||
//} | |||
|
|||
func ToK8SObject(object interface{}, target interface{}) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can move those func to a separate file, not context.go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest a file name?
return json.Unmarshal(bytes, target) | ||
} | ||
|
||
func GetDaemonSetPods(kubeClient client.Client, namespace string, name string) ([]corev1.Pod, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated with utils GetDaemonSetPods.
// } | ||
//} | ||
|
||
func ObjectConversion(object interface{}, target interface{}) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need change Object and ObjectIdenty to seperate file, not context.go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest a filename ?
} | ||
|
||
// will be use in future. | ||
// if no data provider config set, use default history data provider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need grpc data source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can implement it in future.
return fmt.Errorf("%s checkFluctuation failed: %v", rr.Name(), err) | ||
} | ||
|
||
targetUtilization, requestTotal, err := rr.proposeTargetUtilization(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix lint error.
} | ||
|
||
// checkFluctuation check if the time series fluctuation is reach to replicas.fluctuation-threshold | ||
func (rr *HPARecommender) checkFluctuation(medianMin, medianMax float64) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
look like same as replicas function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a hpa recommendation function. Already remove from replicas recommender.
return nil | ||
} | ||
|
||
func (rr *ResourceRecommender) CollectData(ctx *framework.RecommendationContext) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resource no need collect data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently on.
"github.com/gocrane/crane/pkg/utils" | ||
) | ||
|
||
const callerFormat = "RecommendationCaller-%s-%s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as hpa recommender, need change to ResourceRecommederCaller-%s-%s.
const callerFormat = "RecommendationCaller-%s-%s" | ||
|
||
func (rr *ResourceRecommender) PreRecommend(ctx *framework.RecommendationContext) error { | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need load algoritjm config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
every container has two algoritjm config: cpu and memory ,so cannot use context's algoritjm config. we may need to reconsider it.
Signed-off-by: qmhu <brillantroad@gmail.com>
link: #433 |
What type of PR is this?
Feature
What this PR does / why we need it:
Define recommendation framework interfaces .
Implement replicas recommendation via framework.
Special notes for your reviewer:
@qmhu