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.

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

enable configmap creation and bump chart to v0.16.0-rc.2 #834

Merged
merged 5 commits into from
Aug 3, 2023

Conversation

chanwit
Copy link
Collaborator

@chanwit chanwit commented Aug 2, 2023

Fixes #828
Fixes #831

@chanwit chanwit force-pushed the fix-bp-helm-chart-and-bump-to-v0.16.0-rc.2 branch from f09d722 to 5222264 Compare August 2, 2023 13:50
yitsushi
yitsushi previously approved these changes Aug 3, 2023
Copy link
Collaborator

@yitsushi yitsushi left a comment

Choose a reason for hiding this comment

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

lgtm, but

I would still include the following changes:

internal/config/config.go:

-	config := Config{}
+	config := Config{
+		Resources: []types.NamespacedName{
+			{Namespace: RuntimeNamespace()},
+		},
+	}

internal/server/polling/config.go:

 func (s *Server) readConfig(ctx context.Context) (*config.Config, error) {
 	configMap, err := config.ReadConfig(ctx, s.clusterClient, s.configMapRef)
 	if err != nil {
-		return nil, fmt.Errorf("unable to read ConfigMap: %w", err)
+		s.log.Error(err, "failed ot read config")
 	}
 
 	return &configMap, nil

and someone in the informer the same logic.

Reading config can fail only in two cases:

  1. Failed to Get the ConfigMap.
  2. Failed to Unmarshal the content of the ConfigMap.

in both cases, we can use a default value.

yitsushi
yitsushi previously approved these changes Aug 3, 2023
chanwit added 2 commits August 3, 2023 18:56
Signed-off-by: Chanwit Kaewkasi <[email protected]>
Signed-off-by: Chanwit Kaewkasi <[email protected]>
@chanwit chanwit requested a review from yitsushi August 3, 2023 12:02
@chanwit chanwit merged commit 3e3a5f7 into main Aug 3, 2023
@bigkevmcd bigkevmcd deleted the fix-bp-helm-chart-and-bump-to-v0.16.0-rc.2 branch September 14, 2023 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants