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

Add dmsghttp logserver route #1579

Merged
merged 36 commits into from
May 25, 2023
Merged

Add dmsghttp logserver route #1579

merged 36 commits into from
May 25, 2023

Conversation

0pcom
Copy link
Collaborator

@0pcom 0pcom commented May 24, 2023

  • Adds /node-info as a route for the dmsghttp survey & transport logging server
    this route will generate the system survey and respond with it, rather than serving it as a static file.

It will also allow the survey to be collected even if the reward address does not exist

Mainly it is needed for collecting surveys from visors which are otherwise online and once generated a survey, but subsequently are not serving it
#1570

this will also allow for surveys to be collected from the survey whitelisted public keys for management of clusters in the instance the operator is not otherwise seeking rewards

@@ -108,7 +108,8 @@ const (
RewardFile string = "reward.txt"

// NodeInfo is the name of the survey file
NodeInfo string = "node-info.json"
NodeInfo string = "node-info.json"
SurveyName string = "node-info" // SurveyName ...
Copy link
Member

Choose a reason for hiding this comment

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

No need to define the HTTP path here. We dont need to change it frequently and can just hardcode it as its used exactly once.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will change

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've dropped the extra declaration SurveyName, but instead of directly hardcoding I've opted for this implementation:

	//Derive the surveyName from visorconfig.NodeInfo by dropping the ".json" suffix
	surveyName := strings.TrimSuffix(string(visorconfig.NodeInfo), ".json")
	// This survey endpoint generates the survey as a response
	authRoute.GET("/"+surveyName, func(c *gin.Context) {

@@ -54,84 +54,3 @@ func (d *Duration) UnmarshalJSON(b []byte) error {
return errors.New("invalid duration")
}
}

// VisorConfig is every field of the config
type VisorConfig struct {
Copy link
Member

Choose a reason for hiding this comment

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

This is the old visorconfig, right? Any reason we are deleting it? Because its not used anymore and we dont expect users to have to upgrade from that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that was something that I added there a long time ago that i think was used briefly but then not used

rewardAddressBytes, err := os.ReadFile(localPath + "/" + visorconfig.RewardFile) //nolint
if err == nil {
//remove any newline from rewardAddress string
rewardAddress = strings.TrimSuffix(string(rewardAddressBytes), "\n")
Copy link
Member

Choose a reason for hiding this comment

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

Just use strings.TrimSpace please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed on next commit

Comment on lines +80 to +92
// Print results.
s, err := json.MarshalIndent(survey, "", "\t")
if err != nil {
log.WithError(err).Error("Could not marshal system survey to json.")
}

c.Header("Content-Type", "application/json")
c.Writer.WriteHeader(http.StatusOK)

_, err = c.Writer.Write([]byte(s))
if err != nil {
httputil.GetLogger(c.Request).WithError(err).Errorf("failed to write json response")
}
Copy link
Member

Choose a reason for hiding this comment

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

I think I mentioned this before, but you can literally just do:

c.JSON(http.StatusOK, gin.H{survey})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am intentionally using generic c.Writer.Write methods instead of gin's type-specific methods.

It makes it more interchangeable with standard library http server in case we ever decide to move away from gin.

@0pcom
Copy link
Collaborator Author

0pcom commented May 25, 2023

The latest commit also adds an authenticated route for the reward.txt file ; just in case

This will optionally allow for switching from downloading the full survey on every log & survey collection run to optionally just refreshing the reward.txt file ; the surveys can be re-downloaded less frequently in that instance.

@0pcom 0pcom merged commit eb0cfa0 into skycoin:develop May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants