ipfs-operator icon indicating copy to clipboard operation
ipfs-operator copied to clipboard

Add service-config and expose metrics

Open patilsuraj767 opened this issue 2 years ago • 5 comments

fixes: https://github.com/ipfs-cluster/ipfs-operator/issues/95 fixes: https://github.com/ipfs-cluster/ipfs-operator/issues/96

Below are the changes made in this PR

  • Inherited config type from ipfs-cluster
  • Added mechanism to build and modify service.json inside operator.
  • Added Option to enable and expose ipfs-cluster metrics
  • upgraded go version to match ipfs-cluster project
  • upgraded kubo image to match latest

patilsuraj767 avatar Jan 17 '23 07:01 patilsuraj767

Let's get the ginkgo versioning fix in #94 merged into main and then we can rebase this branch and retry the build.

RobotSail avatar Jan 17 '23 19:01 RobotSail

Suggested edit:

diff --git a/controllers/ipfs_cluster_service.go b/controllers/ipfs_cluster_service.go
index cb10c4c..7a2ab9c 100644
--- a/controllers/ipfs_cluster_service.go
+++ b/controllers/ipfs_cluster_service.go
@@ -6,7 +6,7 @@ import (
 )
 
 func GetDefaultServiceConfig() *cmdutils.ConfigHelper {
-	cfgHelper := cmdutils.NewConfigHelper("", "", "crdt", "badger")
+	cfgHelper := cmdutils.NewConfigHelper("", "", "crdt", "flatfs")
 	err := cfgHelper.Manager().Default()
 	if err != nil {
 		return nil

RobotSail avatar Jan 17 '23 20:01 RobotSail

@patilsuraj767 Now that we merged #94, you can rebase this one and it should clear up the build issue as well.

RobotSail avatar Jan 18 '23 15:01 RobotSail

@patilsuraj767 For some reason it's still running the old workflow in CI, you'll have to rebase to get the tests to pass.

RobotSail avatar Jan 23 '23 20:01 RobotSail

@patilsuraj767 This PR looks like it's in working order according to the CI which is good. I only have a few other things that I can think of:

  1. We'll want to have documentation for how the metrics work, how to interface, etc. There's a docs folder at the root of this project where we have a read-the-docs styled documentation page. You'll want to add a section for metrics usage so that it's clear how we can extend them. This doesn't need to be super long, just as long as we know how it's used.
  2. I see that this PR updates the Go version from 1.18 to 1.19. I'm not against this if it's necessary for the docs, but we'll want to split this up into a separate PR so that we have a base commit to refer to in case of any regressions.

Once those two things, I think we should be ready to merge.

RobotSail avatar Jan 30 '23 17:01 RobotSail